-
Notifications
You must be signed in to change notification settings - Fork 2
Add docker build instructions to README #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughUpdated README: replaced docker publish flow to build the image from Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
README.md (2)
230-235
: Environment variable mismatch: use MY_MODEL_IDS to match code and autoscalerCode uses os.environ["MY_MODEL_IDS"] and autoscaler queueNameVars includes MY_MODEL_IDS, but the deployment example sets MODEL_IDS. This will break queue setup and loading.
env: IMPORTS: |- my_model - MODEL_IDS: |- + MY_MODEL_IDS: |- model_id_1 model_id_2Note: If your worker expects a fully qualified module path for IMPORTS (e.g., common.my_model), please confirm and adjust accordingly.
230-232
: Use fully qualified import path for your modelPlease update the example in README.md (lines 230–232) to match how other charts and run-dev.sh expect imports. For example:
- IMPORTS: |- - my_model + IMPORTS: |- + common.my_model MODEL_IDS: |- model_id_1This aligns with:
- The
./scripts/run-dev.sh common common.my_model
example (README.md #145)- All other
IMPORTS:
entries in chart/model-values.yaml (they usecommon.xxx
)- How Celery reads
os.environ["IMPORTS"].split()
in celeryconfig.py
🧹 Nitpick comments (3)
README.md (3)
201-206
: Fix typo and streamline Docker build/tag/push commands
- Typo: “For exaxmple” -> “For example”.
- Build flow: Tag the image with the version at build time and also tag the registry target in one command; then push once. This is cleaner and avoids an extra retag step.
- For exaxmple, if building the 5th version of `gooey-gpu-common` from the `common` directory: + For example, if building the 5th version of `gooey-gpu-common` from the `common` directory: @@ - docker build common -t gooey-gpu-common - docker tag gooey-gpu-common <registry>/gooey-gpu-common:5 - docker push <registry>/gooey-gpu-common:5 + docker build -t gooey-gpu-common:5 -t <registry>/gooey-gpu-common:5 common + docker push <registry>/gooey-gpu-common:5Optional: If you publish for multiple architectures, consider buildx:
- docker buildx build --platform linux/amd64,linux/arm64 -t gooey-gpu-common:5 -t /gooey-gpu-common:5 common --push
223-225
: Align image name with the example you push aboveUsing the same image reference from the push step reduces confusion for readers following the doc end-to-end.
- - name: "common-my-model" - image: "<registry>/<image>:<tag>" + - name: "common-my-model" + image: "<registry>/gooey-gpu-common:5"
213-213
: Fix ordered list numbering (duplicate “12.”)The “Update the model-values.yaml” step should be 11 to avoid duplicate “12.” later in the doc.
-12. Update the `model-values.yaml` file with the new image (or create a new file) +11. Update the `model-values.yaml` file with the new image (or create a new file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (actions)
🔇 Additional comments (1)
README.md (1)
213-219
: LGTM: rabbitmqAutoscaling queueNameVars example is clear and correctThe example shows the correct list form for queueNameVars and matches the env var name used elsewhere. No changes needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README.md (2)
204-206
: Streamline the Docker publish flow and avoid hard-coded version.
- Tag the image at build time to remove the extra docker tag step.
- Parameterize the version to discourage copying a stale "5".
- docker build -f common/Dockerfile -t gooey-gpu-common . - docker tag gooey-gpu-common <registry>/gooey-gpu-common:5 - docker push <registry>/gooey-gpu-common:5 + VERSION=${VERSION:-5} + docker build -f common/Dockerfile -t <registry>/gooey-gpu-common:${VERSION} . + docker push <registry>/gooey-gpu-common:${VERSION}Optional: if you need multi-arch or reproducible builds, consider buildx:
docker buildx build --platform linux/amd64 \ -f common/Dockerfile \ -t <registry>/gooey-gpu-common:${VERSION} \ --push .
213-213
: Fix step numbering and prefer Markdown auto-numbering."12. Update the model-values.yaml..." appears right after step 10 and later "12. Deploy..." creates a duplicate number. Either:
- Renumber this to 11 here and keep subsequent steps incremented, or
- Use Markdown auto-numbering (all items as "1.") to avoid future drift.
-12. Update the `model-values.yaml` file with the new image (or create a new file) +11. Update the `model-values.yaml` file with the new image (or create a new file)Recommendation: switch the entire ordered list to "1." style so GitHub renders correct numbers automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~201-~201: Ensure spelling is correct
Context: ...image to a container registry. For exaxmple, if building the 5th version of `gooey-...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
For exaxmple, if building the 5th version of `gooey-gpu-common` from the `common` directory: | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in user-facing docs ("For exaxmple" → "For example").
Spelling error in a prominent header; fix to maintain polish.
- For exaxmple, if building the 5th version of `gooey-gpu-common` from the `common` directory:
+ For example, if building version 5 of `gooey-gpu-common` from the `common` directory:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
For exaxmple, if building the 5th version of `gooey-gpu-common` from the `common` directory: | |
For example, if building version 5 of `gooey-gpu-common` from the `common` directory: |
🧰 Tools
🪛 LanguageTool
[grammar] ~201-~201: Ensure spelling is correct
Context: ...image to a container registry. For exaxmple, if building the 5th version of `gooey-...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In README.md around lines 201-202, fix the spelling typo in the sentence
beginning "For exaxmple..." by replacing "exaxmple" with "example" so the line
reads "For example, if building the 5th version of `gooey-gpu-common` from the
`common` directory:" and ensure punctuation/spacing remains correct.
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.