fix: remove unnecessary argument from CMD in Dockerfile#138
fix: remove unnecessary argument from CMD in Dockerfile#138SigireddyBalasai wants to merge 1 commit intoa2aproject:mainfrom
Conversation
Summary of ChangesHello @SigireddyBalasai, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Dockerfile by removing an unnecessary argument from the CMD instruction. This change simplifies the command used to run the uvicorn server, ensuring cleaner and potentially more robust execution within the Docker container without altering the core functionality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request removes the -- argument from the CMD instruction in the Dockerfile. While this change is technically correct as uvicorn does not currently conflict with uv run options, I've raised a medium-severity concern about maintainability. It is a best practice to use -- to clearly separate a command runner's options from the command being executed. This improves robustness against future changes in uv. I have suggested reverting this change to adhere to this best practice.
| # The command to run the server. Because our WORKDIR is now /app/backend, | ||
| # we can simply refer to 'app:app' from the current directory.git | ||
| CMD ["uv", "run", "--", "uvicorn", "app:app", "--host", "0.0.0.0", "--port", "8080"] | ||
| CMD ["uv", "run", "uvicorn", "app:app", "--host", "0.0.0.0", "--port", "8080"] |
There was a problem hiding this comment.
While the CMD works without the -- separator, it's a best practice to include it for clarity and robustness. The -- explicitly tells uv run that all subsequent arguments belong to the command being executed (uvicorn), not to uv run itself. This prevents potential issues if future versions of uv introduce options that could conflict with the command or its arguments. Keeping it improves the long-term maintainability of this Dockerfile.
CMD ["uv", "run", "--", "uvicorn", "app:app", "--host", "0.0.0.0", "--port", "8080"]
Description
Thank you for opening a Pull Request!
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
CONTRIBUTINGGuide.fix:which represents bug fixes, and correlates to a SemVer patch.feat:represents a new feature, and correlates to a SemVer minor.feat!:, orfix!:,refactor!:, etc., which represent a breaking change (indicated by the!) and will result in a SemVer major.bash scripts/format.shfrom the repository root to format)Fixes #<issue_number_goes_here> 🦕