Skip to content

Commit

Permalink
chore: cleanup todo comments throughout the codebase (#375)
Browse files Browse the repository at this point in the history
  • Loading branch information
YrrepNoj authored Apr 9, 2024
1 parent c699ff9 commit 1aec77b
Show file tree
Hide file tree
Showing 12 changed files with 7 additions and 21 deletions.
1 change: 0 additions & 1 deletion .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ jobs:
with:
python-version-file: 'pyproject.toml'

# TODO @JPERRY: We should be generating the proto files before this...
- name: Install Python Deps
run: python -m pip install ".[dev,e2e-test]"

Expand Down
1 change: 0 additions & 1 deletion .github/workflows/pytest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ jobs:
with:
python-version-file: 'pyproject.toml'

# TODO @JPERRY: This repeater image is floating in the void. Give it a home in this repo???
- name: Run Repeater
run: docker run -p 50051:50051 -d ghcr.io/defenseunicorns/leapfrogai/repeater:0.3.3

Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ gen-python: ## Generate the protobufs for the OpenAI typing within the leapfroga
--grpc_python_out=src/. \
src/leapfrogai_api/types/proto/leapfrogai_api/types/**/*.proto

# TODO: Make the port a variable with an uncommon default
local-registry: ## Start up a local container registry. Errors in this target are ignored.
-docker run -d -p 5000:5000 --restart=always --name registry registry:2

Expand Down
2 changes: 0 additions & 2 deletions packages/llama-cpp-python/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ ARG REPO_ID=TheBloke/SynthIA-7B-v2.0-GGUF
ARG FILENAME=synthia-7b-v2.0.Q4_K_M.gguf
ARG REVISION=3f65d882253d1f15a113dabf473a7c02a004d2b5

# TODO: Standardize on a better approach for how we download models into our images across our packages
# NOTE: This is checking for a pre-downloaded model file in the local build dir before downloading the model from HuggingFace
# TODO: Add checksum validation to verify the model in the local build-dir is the model we expect
COPY scripts/model_download.py scripts/model_download.py
Expand All @@ -25,7 +24,6 @@ ENV PATH="/leapfrogai/.venv/bin:$PATH"

# copy the llama-cpp-python build dependencies over
# NOTE: We are copying to this filename because installing 'optional extras' from a wheel requires the absolute path to the wheel file (instead of a wildcard whl)
# TODO: This install seems slow. Evaluate options such as copying ALL of the whl's to this image OR vendoring/bundling the dependencies inside of our built .whl
COPY build/*.whl build/
COPY build/leapfrogai_api*.whl leapfrogai_api-100.100.100-py3-none-any.whl
RUN pip install leapfrogai_api-100.100.100-py3-none-any.whl[llama-cpp-python] --no-index --find-links=build/
Expand Down
2 changes: 1 addition & 1 deletion packages/llama-cpp-python/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ components:
namespace: leapfrogai
localPath: chart
releaseName: llama-cpp-python-model
version: 0.0.1 #TODO: Validate this package version
version: 0.0.1
valuesFiles:
- "llama-cpp-python-values.yaml"
images:
Expand Down
2 changes: 1 addition & 1 deletion packages/text-embeddings/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ components:
namespace: leapfrogai
localPath: chart
releaseName: text-embeddings-model
version: 0.0.1 #TODO: Validate this package version
version: 0.0.1
valuesFiles:
- "embedding-values.yaml"
images:
Expand Down
5 changes: 1 addition & 4 deletions packages/vllm/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@ RUN groupadd -g 65532 vglusers && \
WORKDIR /home/leapfrogai

# grab necesary python dependencies
# TODO @JPERRY: Get context as to why we are doing this for this Dockerfile but not our other ones
RUN apt-get -y update \
&& apt-get -y install software-properties-common \
&& add-apt-repository universe \
&& add-apt-repository ppa:deadsnakes/ppa \
&& apt-get -y update


# TODO @JPERRY: Install python 3.11.6 specifically by installing it via pyenv...
# TODO @JPERRY: Determine if we really need to be installing 'git' here
RUN apt-get -y install python3.11-full

# get deps for vllm compilation and model download
Expand Down Expand Up @@ -53,4 +50,4 @@ COPY config.yaml .

EXPOSE 50051:50051

ENTRYPOINT ["python3.11", "-m", "leapfrogai_api.types.cli", "--app-dir=.", "main:Model"]
ENTRYPOINT ["python3.11", "-m", "leapfrogai_api.types.cli", "--app-dir=.", "main:Model"]
2 changes: 1 addition & 1 deletion packages/vllm/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ components:
namespace: leapfrogai
localPath: chart
releaseName: vllm-model
version: 0.0.1 #TODO: Validate this package version
version: 0.0.1
valuesFiles:
- "vllm-values.yaml"
images:
Expand Down
4 changes: 1 addition & 3 deletions packages/whisper/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ RUN python3.11 -m venv .venv
ENV PATH="/leapfrogai/.venv/bin:$PATH"

# download and covnert OpenAI's whisper base
# TODO @JPERRY: Move this model download to a separate build stage (within its own FROM section)
# We don't dirty our .venv with libraries only needed for model download...
ARG MODEL_NAME=openai/whisper-base
RUN pip install transformers[torch]
RUN pip install ctranslate2
Expand Down Expand Up @@ -45,4 +43,4 @@ COPY main.py .

EXPOSE 50051:50051

ENTRYPOINT ["python", "-u", "main.py"]
ENTRYPOINT ["python", "-u", "main.py"]
1 change: 0 additions & 1 deletion packages/whisper/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@


def make_transcribe_request(filename, task, language, temperature, prompt):
# TODO @JPERRY does this make sense to be in the function? This seems like something that should be done during initialization
device = "cuda" if GPU_ENABLED else "cpu"
model = WhisperModel(model_path, device=device, compute_type="float32")

Expand Down
2 changes: 1 addition & 1 deletion packages/whisper/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ components:
namespace: leapfrogai
localPath: chart
releaseName: whisper-model
version: 0.0.1 #TODO: Validate this package version
version: 0.0.1
valuesFiles:
- "whisper-values.yaml"
images:
Expand Down
5 changes: 1 addition & 4 deletions src/leapfrogai_api/backends/openai/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,14 @@ class ChatStreamChoice(BaseModel):
finish_reason: str | None = ""


# TODO @JPERRY do we want two distinct response types for stream vs not-stream or do we want the choices to be unioned?
class ChatCompletionResponse(BaseModel):
"""https://platform.openai.com/docs/api-reference/chat/object"""

id: str = ""
object: str = "chat.completion"
created: int = 0
model: str = ""
choices: (
list[ChatChoice] | list[ChatStreamChoice]
) # TODO: @JPERRY look into this more, difference between streaming and not streaming
choices: list[ChatChoice] | list[ChatStreamChoice]
usage: Usage | None = None


Expand Down

0 comments on commit 1aec77b

Please sign in to comment.