-
Notifications
You must be signed in to change notification settings - Fork 4.1k
fix: updated docker file to use lock file if it's present, if it's no… #34833
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: release
Are you sure you want to change the base?
Conversation
…t then to use package.json to install dependencies.
Important Review skippedAuto reviews are limited to specific labels. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Dockerfile changes focus on improving the structure and efficiency of the container setup process. Enhancements include streamlining the creation and management of directories, adjusting how files are copied and permissions are set, updating environment variables, and modifying the health check command to ensure better reliability and observability of the service. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Dockerfile (4 hunks)
Additional comments not posted (7)
Dockerfile (7)
3-3
: LGTM! Explicitly naming the base stage improves clarity.The base stage is explicitly named "base", which is a good practice for multi-stage builds.
16-16
: LGTM! Creating necessary directories ensures smooth file operations.The directories for plugins, editor, and RTS are created to ensure they exist before copying files into them.
18-19
: LGTM! Copying necessary files ensures they are available in the container.The JAR files, client UI, RTS, and package files are copied into the container to ensure they are available for the application.
Also applies to: 30-30
32-32
: LGTM! Updating the PATH variable ensures correct directories are included.The PATH environment variable is updated to include necessary directories.
36-42
: LGTM! Conditional dependency installation ensures correct setup.Dependencies are installed using
yarn ci
oryarn install
based on the presence ofpackage-lock.json
, ensuring the correct setup.
44-47
: LGTM! Additional setup and permissions ensure correct configuration.Additional setup and permissions are configured for npm installation, script execution, and directory access.
59-60
: LGTM! Health check and entry point command updates ensure correct management.The health check command is updated to ensure correct monitoring, and the entry point command is commented out to manage it appropriately.
Hello @srix @ayushpahwa , Thank you. |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Hii @srix @ayushpahwa , thank you. |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Hii @srix @ayushpahwa , thank you. |
Hii @srix @ayushpahwa @Nikhil-Nandagopal @rohan-arthur , thank you. |
I dont see my earlier comments resolved |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Description
this is the PR for the Isuue - #33743
Updates in Pr:
Description about the chnaged code:
1.the new code main check for the lock file , if its present then it install the depencies from lock file
, if its not there then it install the dependecies through the package.json file.
the new code main deals with this also updated few lines to avoid the errors while generting the image
for the project.
2.EntryPoint is commentted and its written in the RUN along by providing the permission to the file
""chmod +x /deploy/docker/fs/entrypoint.sh "".
3.In Docker, it's a good practice to use /opt/bin as a directory to store custom binaries and scripts
that you want to include in your Docker image. This directory is then added to the PATH environment
variable, allowing the executables within it to be easily run from anywhere in the container.
In the code, I have used /opt instead of /opt/bin. There is not much difference between them, and it
does not cause any errors. Both /opt and /opt/bin are acceptable, but /opt/bin is commonly used for
rganizing custom executables.
I have tested it locally with appsmith project as well as other project containing lock file.It was working fine.
Below are the screenshots attached from build docker image to running the container and listing the files inside the image:
@srix @ayushpahwa
Summary by CodeRabbit