feat: Make images adaptable to support both Postgres and MongoDB uris #36424
Conversation
WalkthroughThis pull request introduces several modifications across multiple files to enhance the project's build process and configuration management. Changes include the addition of SQL file support in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (3)
Files not reviewed due to no reviewable changes (1)
Additional comments not posted (5)
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10939327092. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
scripts/prepare_server_artifacts.sh (1)
12-12: Setting a default value forPG_TAGis a smart move! 🧠Defaulting
PG_TAGto "pg" if it's not already defined ensures that the script can continue executing even if the variable isn't explicitly set. This adds robustness to the script.However, it's not immediately clear what
PG_TAGrepresents or what other values it might take.To improve the script's readability and maintainability, consider adding a brief comment explaining the purpose and possible values of
PG_TAG. This will help future maintainers understand the script's behavior more easily.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- .editorconfig (1 hunks)
- .github/workflows/build-docker-image.yml (1 hunks)
- Dockerfile (1 hunks)
- deploy/docker/fs/opt/appsmith/entrypoint.sh (1 hunks)
- scripts/local_testing.sh (1 hunks)
- scripts/prepare_server_artifacts.sh (1 hunks)
Additional comments not posted (11)
.editorconfig (1)
12-12: Great work, class! 👏Adding
*.sqlto the configuration forindent_size = 4is a fantastic idea. It ensures that our SQL files will follow the same indentation rules as our Java, XML, and Python files. This change will make our codebase more consistent and easier to read.Let's give ourselves a pat on the back for promoting good coding practices! 🎉
scripts/prepare_server_artifacts.sh (4)
1-1: Great job using the appropriate shebang and error handling options! 👍The shebang
#!/bin/bash -euxcorrectly specifies Bash as the interpreter and enables important error handling and debugging options. This will help ensure the script runs reliably and can be easily debugged if needed. Keep up the good work!
5-10: The edition determination logic is spot-on! 🎯I like how you've used the
EDITIONenvironment variable as the primary way to specify the edition, with a smart fallback to checking the Git remote URL. This allows for flexibility and automation in determining the correct edition. Your attention to detail is commendable!
15-17: The target directory preparation is handled perfectly! 💯Creating the
targetdirectory if it doesn't exist is crucial to avoid errors when copying files later in the script. Additionally, removing any existingpgandmongodirectories ensures that old artifacts don't interfere with the new ones being copied.Your script demonstrates excellent attention to detail and defensive programming practices. Well done!
19-28: The server artifact copying logic is exemplary! 🌟I'm impressed by your use of Docker to pull the PostgreSQL server artifacts based on the determined edition and tag. This allows for great flexibility and adaptability in using different configurations.
Running the Docker container in detached mode and copying the necessary files from it is an efficient and clean approach. I also appreciate that you've taken care to clean up the Docker container and image after the artifacts are copied, to avoid leaving behind unnecessary resources.
Your script showcases a deep understanding of Docker and artifact management. Fantastic work!
Dockerfile (2)
15-18: Well done, class! This check is a splendid addition.The code segment correctly verifies the presence of the
info.jsonfile and handles the case when it's missing by outputting an error message and exiting with a non-zero status code. This ensures that the build process fails early if the required file is not found, preventing potential issues down the line.
20-23: Excellent work, everyone! This check is a valuable safeguard.The code segment effectively verifies the presence of both
server.jarfiles for MongoDB and PostgreSQL. If either of the files is missing, it outputs an informative error message suggesting the use of the build script and exits with a non-zero status code. This ensures that the build process fails early if the required files are not found, preventing potential issues and guiding the user to use the appropriate build process.scripts/local_testing.sh (1)
87-88: Great job adding theprepare_server_artifacts.shscript invocation! 👍Class, pay attention here. The addition of the
./scripts/prepare_server_artifacts.shcommand right after thepopdis a smart move. It ensures that the necessary server artifacts are prepared immediately after the server build process. This enhances the overall build workflow by generating the required artifacts at the appropriate stage. Well done!.github/workflows/build-docker-image.yml (1)
82-87: Excellent work on adding the new job step to place server artifacts! 👍This step is a valuable addition to the workflow as it enhances the build process by incorporating artifact management. By running the
prepare_server_artifacts.shscript, the step ensures that the server artifacts specific to the "es" edition are properly placed.The inclusion of this step not only expands the capabilities of the workflow but also modifies the control flow by introducing a new operation after the existing script execution. This change demonstrates a well-thought-out approach to improving the overall build process.
Keep up the great work in enhancing the adaptability and flexibility of the release image! 🌟
deploy/docker/fs/opt/appsmith/entrypoint.sh (2)
375-377: Excellent work on the conditional MongoDB configuration! 🌟Students, take note of the careful consideration given to the MongoDB configuration. The script wisely checks two important conditions before copying the configuration file:
- The URI is local (
$isUriLocal -eq 0).- The MongoDB URL is valid (
$isMongoUrl -eq 1).By ensuring both conditions are met, the script prevents potential misconfiguration issues when the MongoDB URL is not suitable for local use. This attention to detail enhances the robustness of the setup process. Well done!
375-375: Verify the impact of disabling Postgres initialization.Class, it appears that the initialization of Postgres has been commented out when not running in a Heroku dyno environment. Let's put on our thinking caps and ponder the implications of this change.
To verify the impact, please run the following script:
If the tests return no results, it would confirm that Postgres is no longer being used in this context. However, if there are occurrences, further analysis is needed to understand how the application is impacted.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10940047273. |
|
Deploy-Preview-URL: https://ce-36424.dp.appsmith.com |
…er based on DB url
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10942248519. |
|
Deploy-Preview-URL: https://ce-36424.dp.appsmith.com |
abhvsn
changed the title
abhvsn
changed the title
github-actions
bot
added
Enhancement
…appsmithorg#36424) ## Description PR to make the release tag adaptable to work with both MongoDB and PostgreSQL uris. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new script to automate the preparation of server artifacts, improving the build process. - Added SQL files to the indentation configuration for consistent code formatting. - **Improvements** - Enhanced error handling in the Docker build process to ensure essential files are present before execution. - Updated service configuration logic to prevent misconfiguration based on the environment. - Added a new job step in the build workflow to prepare server artifacts after the build process. - Implemented conditional logic in the run script to dynamically adapt to different database configurations. - **Bug Fixes** - Adjusted the initialization process to focus on MongoDB, improving reliability in various environments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> /test Sanity ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10940528231> > Commit: 32731e8 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10940528231&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Thu, 19 Sep 2024 12:21:54 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No
Description
PR to make the release tag adaptable to work with both MongoDB and PostgreSQL uris.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
/test Sanity
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10940528231
Commit: 32731e8
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 19 Sep 2024 12:21:54 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?