chore: add postgres db during the startup conditionally #37605
Conversation
github-actions
bot
added
the
skip-changelog
WalkthroughThe pull request updates the CI workflow configuration to conditionally include a PostgreSQL service while excluding the MongoDB service when PostgreSQL is present. The job execution conditions have been refined to trigger on specific events, and the logic for running tests and restoring cache has been clarified. The handling of Cypress test results and the logging of artifacts upon failure have been preserved, enhancing overall control flow and error handling in the CI process. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/ci-test-limited.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/ci-test-limited.yml
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
🔇 Additional comments (1)
.github/workflows/ci-test-limited.yml (1)
Line range hint 306-306: Verify application's database selection mechanism
The workflow assumes dynamic database selection, but we need to verify how the application handles this.
Let's check the application configuration:
✅ Verification successful
Let me search for database configuration files to verify the database selection mechanism.
Let me check the database configuration implementation to understand how the database selection works.
The POSTGRES_PASSWORD environment variable is correctly handled by the application's database configuration system
The codebase shows a robust database connection management system through:
- Dedicated datasource configuration classes that handle database connections
- Plugin-based architecture where each database type (including Postgres) has its own connection management
- Environment-aware configuration that can handle dynamic database parameters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for database configuration files
echo "Searching for database configuration files..."
fd -e yml -e yaml -e properties -e xml -e json -e js -e ts | grep -i "database\|config"
# Search for database connection logic
echo "Searching for database connection code..."
rg -l "mongoose\.connect|createConnection|pg\.Pool|new\s+Client"
Length of output: 34387
Script:
#!/bin/bash
# Search for database configuration files in server code
echo "Searching for database configuration files..."
fd -e yml -e yaml -e properties -e xml -e json -e java | grep -i "database\|datasource"
# Search for database selection logic
echo "Searching for database selection code..."
rg -l "datasource.*config|database.*config" --type java --type ts
Length of output: 22664
Script:
#!/bin/bash
# Check MongoDB configuration
echo "Checking MongoDB configuration..."
rg -A 10 "class MongoConfig" --type java
# Check database connection setup
echo "Checking database connection setup..."
rg -A 10 "datasource.*connection|database.*connection" --type java
Length of output: 116065
🧰 Tools
🪛 actionlint
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
| mongo: | ||
| if: "!contains(github.base_ref, 'pg') && github.ref_name != 'pg'" |
There was a problem hiding this comment.
Fix invalid service-level condition syntax
GitHub Actions doesn't support conditional service execution using the if key. Consider using environment variables or job-level conditions instead.
Consider this alternative approach:
- mongo:
- if: "!contains(github.base_ref, 'pg') && github.ref_name != 'pg'"
- image: mongo
- ports:
- - 27017:27017
+ mongo:
+ image: mongo
+ ports:
+ - 27017:27017Then control the database selection through environment variables in the application configuration.
📝 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.
| if: "!contains(github.base_ref, 'pg') && github.ref_name != 'pg'" | |
| mongo: | |
| image: mongo | |
| ports: | |
| - 27017:27017 |
🧰 Tools
🪛 actionlint
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
| postgres: | ||
| if: contains(github.base_ref, 'pg') || github.ref_name == 'pg' | ||
| image: postgres:14 | ||
| ports: | ||
| - 5432:5432 |
There was a problem hiding this comment.
Add PostgreSQL environment variables and restrict branch conditions
The PostgreSQL service configuration needs attention in two areas:
- Missing essential environment variables (POSTGRES_USER, POSTGRES_PASSWORD)
- Branch name condition might unintentionally trigger for branches containing 'pg'
Apply this diff to enhance the configuration:
postgres:
if: contains(github.base_ref, 'pg') || github.ref_name == 'pg'
image: postgres:14
+ env:
+ POSTGRES_USER: postgres
+ POSTGRES_PASSWORD: ${{ secrets.POSTGRES_PASSWORD }}
ports:
- 5432:5432Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/ci-test-limited.yml (1)
51-57: Consider using environment files for database selectionTo improve the workflow's maintainability and avoid service-level conditions, consider using environment files to control database selection.
Example approach:
- Create a step that determines which database to use
- Set environment variables based on the branch
- Use these variables in subsequent steps
steps: - name: Set database type run: | if [[ "${{ github.ref }}" == "refs/heads/pg" ]]; then echo "USE_POSTGRES=true" >> $GITHUB_ENV echo "USE_MONGO=false" >> $GITHUB_ENV else echo "USE_POSTGRES=false" >> $GITHUB_ENV echo "USE_MONGO=true" >> $GITHUB_ENV fi🧰 Tools
🪛 actionlint (1.7.4)
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/ci-test-limited.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-limited.yml
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
🔇 Additional comments (1)
.github/workflows/ci-test-limited.yml (1)
57-57:
Fix invalid MongoDB service condition syntax
GitHub Actions doesn't support conditional service execution using the if key at the service level.
The previous review comment about moving conditions to job-level environment variables is still valid. This ensures proper control over which database service to use during the workflow execution.
🧰 Tools
🪛 actionlint (1.7.4)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
| mongo: | ||
| if: "!contains(github.base_ref, 'pg') && github.ref_name != 'pg'" |
There was a problem hiding this comment.
@AnaghHegde can we remove the contains here as well?
AnaghHegde
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/ci-test-limited.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-test-limited.yml
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
🔇 Additional comments (2)
.github/workflows/ci-test-limited.yml (2)
57-57:
Fix invalid service-level condition syntax for MongoDB
GitHub Actions doesn't support conditional service execution using the if key.
Move the condition logic to the job level using environment variables:
jobs:
ci-test-limited:
env:
USE_MONGO: ${{ github.base_ref == 'release' || github.ref_name == 'release' }}🧰 Tools
🪛 actionlint (1.7.4)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
51-57: Verify database service configuration
Let's verify the database service configuration across the codebase to ensure proper integration.
✅ Verification successful
Let me gather more specific information about the database configurations in the workflow files.
The initial results show both MongoDB and PostgreSQL usage, but I need to verify the specific configurations in the workflow files and their alignment with the codebase setup.
Database service configuration is correctly aligned
The workflow configuration for both PostgreSQL and MongoDB services is properly aligned with the codebase's requirements. The conditional service activation (if: github.base_ref == 'pg' for PostgreSQL and if: github.base_ref == 'release' for MongoDB) matches the application's dual database support, as evidenced by:
- MongoDB configuration in dev/test environments (
mongodb://localhost:27017/appsmith) - PostgreSQL plugin implementation with proper connection handling
- Presence of both database plugins and their respective connection utilities
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for database configuration and usage patterns
# Check for database configuration patterns
echo "Checking for database configuration patterns..."
rg -l "mongodb://" || echo "No MongoDB connection strings found"
rg -l "postgresql://" || echo "No PostgreSQL connection strings found"
# Check for database environment variables
echo "Checking for database environment variables..."
rg -l "POSTGRES_" || echo "No PostgreSQL environment variables found"
rg -l "MONGO_" || echo "No MongoDB environment variables found"
# Check for database dependencies
echo "Checking for database dependencies..."
fd -t f "package.json" -x jq -r '.dependencies | select(.mongoose or .pg)'
Length of output: 4797
Script:
#!/bin/bash
# Check workflow files for database service configurations
echo "Checking workflow files for database configurations..."
fd -e yml -e yaml -p ".github/workflows" -x cat {}
# Check for database-related environment files
echo "Checking database configuration in environment files..."
fd -e env -e "env.example" -x cat {}
# Check for database connection setup in server code
echo "Looking for database connection setup..."
rg -A 5 "createConnection|DatabaseConfig" app/server/
Length of output: 18430
🧰 Tools
🪛 actionlint (1.7.4)
52-52: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
57-57: unexpected key "if" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
Description
This pull request proposes a change to conditionally add a PostgreSQL database during startup.
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12064918747
Commit: fbded31
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 28 Nov 2024 08:44:27 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit