fixed pr workflow which ensures that talawa Api app starts in docker #2653
Conversation
WalkthroughThe changes in this pull request enhance the GitHub Actions workflow defined in Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
.github/workflows/pull-request.yml
Outdated
|
|
||
| - name: Checks Talawa API app starts in Docker | ||
| run: | | ||
| if [[ "${{ runner.os }}" == "Linux" ]]; then | ||
| bash sudo /usr/libexec/docker/cli-plugins/docker-compose -f docker-compose.dev.yaml up --build | ||
| elif [[ "${{ runner.os }}" == "Windows" ]]; then | ||
| docker-compose -f docker-compose.dev.yaml up --build | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve workflow structure to avoid resource conflicts
The current workflow runs both Docker containers and local development server in the same job, which could lead to port conflicts and redundant testing. Consider restructuring the workflow for better separation of concerns.
Suggestions:
- Split into separate jobs:
Test-Application: Run tests locally (existing)Docker-Check: Run Docker validation (new)
- Add job dependency:
Docker-Check: needs: [Test-Application] # ... Docker check implementation
This separation will:
- Prevent port conflicts
- Make the workflow more maintainable
- Provide clearer failure points
- Allow parallel execution if needed
.github/workflows/pull-request.yml
Outdated
| - name: Checks Talawa API app starts in Docker | ||
| run: | | ||
| if [[ "${{ runner.os }}" == "Linux" ]]; then | ||
| bash sudo /usr/libexec/docker/cli-plugins/docker-compose -f docker-compose.dev.yaml up --build | ||
| elif [[ "${{ runner.os }}" == "Windows" ]]; then | ||
| docker-compose -f docker-compose.dev.yaml up --build | ||
| fi |
There was a problem hiding this comment.
Critical issues in Docker startup check implementation
The current implementation has several issues that need to be addressed:
- The Docker Compose command path is incorrect and non-standard
- Missing detached mode will cause the workflow to hang
- No health check to verify successful startup
- No timeout handling
- No cleanup of Docker resources
- Unnecessary Windows condition (GitHub Actions uses Ubuntu runners)
Replace the implementation with this more robust version:
- - name: Checks Talawa API app starts in Docker
- run: |
- if [[ "${{ runner.os }}" == "Linux" ]]; then
- bash sudo /usr/libexec/docker/cli-plugins/docker-compose -f docker-compose.dev.yaml up --build
- elif [[ "${{ runner.os }}" == "Windows" ]]; then
- docker-compose -f docker-compose.dev.yaml up --build
- fi
+ - name: Check if Talawa API starts in Docker
+ run: |
+ # Start containers in detached mode
+ docker-compose -f docker-compose.dev.yaml up -d --build
+
+ # Wait for the API to be healthy (max 60 seconds)
+ timeout=60
+ until curl -f http://localhost:4000/health || [ $timeout -eq 0 ]; do
+ echo "Waiting for API to start... ($timeout seconds remaining)"
+ sleep 1
+ ((timeout--))
+ done
+
+ if [ $timeout -eq 0 ]; then
+ echo "Error: API failed to start within timeout"
+ docker-compose -f docker-compose.dev.yaml logs
+ exit 1
+ fi
+
+ echo "API started successfully"
+
+ # Cleanup
+ docker-compose -f docker-compose.dev.yaml down -vThis implementation:
- Uses standard
docker-composecommand - Runs containers in detached mode
- Implements proper health check with timeout
- Shows container logs on failure
- Cleans up resources after check
📝 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.
| - name: Checks Talawa API app starts in Docker | |
| run: | | |
| if [[ "${{ runner.os }}" == "Linux" ]]; then | |
| bash sudo /usr/libexec/docker/cli-plugins/docker-compose -f docker-compose.dev.yaml up --build | |
| elif [[ "${{ runner.os }}" == "Windows" ]]; then | |
| docker-compose -f docker-compose.dev.yaml up --build | |
| fi | |
| - name: Check if Talawa API starts in Docker | |
| run: | | |
| # Start containers in detached mode | |
| docker-compose -f docker-compose.dev.yaml up -d --build | |
| # Wait for the API to be healthy (max 60 seconds) | |
| timeout=60 | |
| until curl -f http://localhost:4000/health || [ $timeout -eq 0 ]; do | |
| echo "Waiting for API to start... ($timeout seconds remaining)" | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: API failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| exit 1 | |
| fi | |
| echo "API started successfully" | |
| # Cleanup | |
| docker-compose -f docker-compose.dev.yaml down -v |
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/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/pull-request.yml
299-299: "runs-on" section is missing in job "Docker-Check"
(syntax-check)
🪛 yamllint
.github/workflows/pull-request.yml
[error] 298-298: trailing spaces
(trailing-spaces)
[error] 301-301: trailing spaces
(trailing-spaces)
[error] 305-305: trailing spaces
(trailing-spaces)
[error] 312-312: trailing spaces
(trailing-spaces)
[error] 318-318: trailing spaces
(trailing-spaces)
[error] 320-320: trailing spaces
(trailing-spaces)
.github/workflows/pull-request.yml
Outdated
| - name: Check if Talawa API starts in Docker | ||
| run: | | ||
| docker-compose -f docker-compose.dev.yaml up -d --build | ||
|
|
||
| timeout=60 | ||
| until curl -f http://localhost:4000/health || [ $timeout -eq 0 ]; do | ||
| echo "Waiting for API to start... ($timeout seconds remaining)" | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done | ||
|
|
||
| if [ $timeout -eq 0 ]; then | ||
| echo "Error: API failed to start within timeout" | ||
| docker-compose -f docker-compose.dev.yaml logs | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "API started successfully" | ||
|
|
||
| # Cleanup | ||
| docker-compose -f docker-compose.dev.yaml down -v |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance Docker check reliability
While the implementation is good, there are a few improvements needed:
- Add error handling for docker-compose commands
- Use environment variable for health check URL
- Ensure no port conflicts with Test-Application job
Apply this diff to enhance the implementation:
+ name: Check if Talawa API starts in Docker
run: |
+ # Ensure no containers are running
+ docker-compose -f docker-compose.dev.yaml down -v || true
+
+ # Start containers
docker-compose -f docker-compose.dev.yaml up -d --build
+ if [ $? -ne 0 ]; then
+ echo "Failed to start containers"
+ exit 1
+ fi
timeout=60
- until curl -f http://localhost:4000/health || [ $timeout -eq 0 ]; do
+ until curl -f "${HEALTH_CHECK_URL:-http://localhost:4000}/health" || [ $timeout -eq 0 ]; do
echo "Waiting for API to start... ($timeout seconds remaining)"
sleep 1
((timeout--))
done
if [ $timeout -eq 0 ]; then
echo "Error: API failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs
+ docker-compose -f docker-compose.dev.yaml down -v
exit 1
fi
echo "API started successfully"
# Cleanup
docker-compose -f docker-compose.dev.yaml down -v
+ if [ $? -ne 0 ]; then
+ echo "Failed to cleanup containers"
+ exit 1
+ fiAdditionally, consider adding these environment variables to the job:
env:
HEALTH_CHECK_URL: http://localhost:4000
COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}The COMPOSE_PROJECT_NAME ensures unique container names per PR, preventing conflicts with other jobs.
🧰 Tools
🪛 yamllint
[error] 305-305: trailing spaces
(trailing-spaces)
[error] 312-312: trailing spaces
(trailing-spaces)
[error] 318-318: trailing spaces
(trailing-spaces)
[error] 320-320: trailing spaces
(trailing-spaces)
.github/workflows/pull-request.yml
Outdated
| Docker-Check: | ||
| needs: Test-Application | ||
| steps: |
There was a problem hiding this comment.
Add missing required "runs-on" field
The job definition is missing the required "runs-on" field which specifies the runner environment. This will cause the workflow to fail.
Apply this diff to fix the job definition:
Docker-Check:
needs: Test-Application
+ runs-on: ubuntu-latest
steps:📝 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.
| Docker-Check: | |
| needs: Test-Application | |
| steps: | |
| Docker-Check: | |
| needs: Test-Application | |
| runs-on: ubuntu-latest | |
| steps: |
🧰 Tools
🪛 actionlint
299-299: "runs-on" section is missing in job "Docker-Check"
(syntax-check)
🪛 yamllint
[error] 301-301: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/pull-request.yml
304-304: shellcheck reported issue in this script: SC2181:style:6:7: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
304-304: shellcheck reported issue in this script: SC2181:style:29:7: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint
.github/workflows/pull-request.yml
[error] 298-298: trailing spaces
(trailing-spaces)
[error] 302-302: trailing spaces
(trailing-spaces)
[error] 320-320: trailing spaces
(trailing-spaces)
[error] 339-339: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/pull-request.yml (1)
299-340: Verify workflow execution order and failure handling
The job integration looks good, but let's verify the workflow execution order and failure handling.
✅ Verification successful
Workflow execution order and failure handling are correctly implemented
Docker-Checkjob properly depends onTest-Application.- Cleanup steps are executed in case of failure.
- Proper error handling with
exit 1is in place.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify workflow execution order and failure handling
# Expected: Docker-Check job should run after Test-Application and handle failures correctly
# Check if Docker-Check depends on Test-Application
rg -A 5 "Docker-Check:" .github/workflows/pull-request.yml
# Check if cleanup is performed in case of failure
rg -A 10 "docker-compose.*down" .github/workflows/pull-request.yml
# Check for proper error handling
rg "exit 1" .github/workflows/pull-request.yml
Length of output: 1357
🧰 Tools
🪛 actionlint
304-304: shellcheck reported issue in this script: SC2181:style:6:7: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
304-304: shellcheck reported issue in this script: SC2181:style:29:7: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint
[error] 302-302: trailing spaces
(trailing-spaces)
[error] 320-320: trailing spaces
(trailing-spaces)
[error] 339-339: trailing spaces
(trailing-spaces)
.github/workflows/pull-request.yml
Outdated
| - name: Check if Talawa API starts in Docker | ||
| run: | | ||
| # Ensure no containers are running | ||
| docker-compose -f docker-compose.dev.yaml down -v || true | ||
|
|
||
| # Start containers | ||
| docker-compose -f docker-compose.dev.yaml up -d --build | ||
| if [ $? -ne 0 ]; then | ||
| echo "Failed to start containers" | ||
| exit 1 | ||
| fi | ||
|
|
||
| timeout=60 | ||
| until curl -f "${HEALTH_CHECK_URL:-http://localhost:4000}/health" || [ $timeout -eq 0 ]; do | ||
| echo "Waiting for API to start... ($timeout seconds remaining)" | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done | ||
|
|
||
| if [ $timeout -eq 0 ]; then | ||
| echo "Error: API failed to start within timeout" | ||
| docker-compose -f docker-compose.dev.yaml logs | ||
| docker-compose -f docker-compose.dev.yaml down -v | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "API started successfully" | ||
|
|
||
| # Cleanup | ||
| docker-compose -f docker-compose.dev.yaml down -v | ||
| if [ $? -ne 0 ]; then | ||
| echo "Failed to cleanup containers" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve shell script robustness
While the implementation is good, there are some improvements needed:
- Use direct command checking instead of
$? - Fix indentation inconsistencies
- Add error handling for curl command
Apply this diff:
- name: Check if Talawa API starts in Docker
run: |
# Ensure no containers are running
docker-compose -f docker-compose.dev.yaml down -v || true
# Start containers
- docker-compose -f docker-compose.dev.yaml up -d --build
- if [ $? -ne 0 ]; then
+ if ! docker-compose -f docker-compose.dev.yaml up -d --build; then
echo "Failed to start containers"
exit 1
fi
timeout=60
- until curl -f "${HEALTH_CHECK_URL:-http://localhost:4000}/health" || [ $timeout -eq 0 ]; do
+ until curl -sf "${HEALTH_CHECK_URL:-http://localhost:4000}/health" 2>/dev/null || [ $timeout -eq 0 ]; do
echo "Waiting for API to start... ($timeout seconds remaining)"
sleep 1
((timeout--))
done
if [ $timeout -eq 0 ]; then
echo "Error: API failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs
docker-compose -f docker-compose.dev.yaml down -v
exit 1
fi
echo "API started successfully"
# Cleanup
- docker-compose -f docker-compose.dev.yaml down -v
- if [ $? -ne 0 ]; then
+ if ! docker-compose -f docker-compose.dev.yaml down -v; then
echo "Failed to cleanup containers"
exit 1
fi📝 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.
| - name: Check if Talawa API starts in Docker | |
| run: | | |
| # Ensure no containers are running | |
| docker-compose -f docker-compose.dev.yaml down -v || true | |
| # Start containers | |
| docker-compose -f docker-compose.dev.yaml up -d --build | |
| if [ $? -ne 0 ]; then | |
| echo "Failed to start containers" | |
| exit 1 | |
| fi | |
| timeout=60 | |
| until curl -f "${HEALTH_CHECK_URL:-http://localhost:4000}/health" || [ $timeout -eq 0 ]; do | |
| echo "Waiting for API to start... ($timeout seconds remaining)" | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: API failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi | |
| echo "API started successfully" | |
| # Cleanup | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| if [ $? -ne 0 ]; then | |
| echo "Failed to cleanup containers" | |
| exit 1 | |
| fi | |
| - name: Check if Talawa API starts in Docker | |
| run: | | |
| # Ensure no containers are running | |
| docker-compose -f docker-compose.dev.yaml down -v || true | |
| # Start containers | |
| if ! docker-compose -f docker-compose.dev.yaml up -d --build; then | |
| echo "Failed to start containers" | |
| exit 1 | |
| fi | |
| timeout=60 | |
| until curl -sf "${HEALTH_CHECK_URL:-http://localhost:4000}/health" 2>/dev/null || [ $timeout -eq 0 ]; do | |
| echo "Waiting for API to start... ($timeout seconds remaining)" | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: API failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi | |
| echo "API started successfully" | |
| # Cleanup | |
| if ! docker-compose -f docker-compose.dev.yaml down -v; then | |
| echo "Failed to cleanup containers" | |
| exit 1 | |
| fi |
🧰 Tools
🪛 actionlint
304-304: shellcheck reported issue in this script: SC2181:style:6:7: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
304-304: shellcheck reported issue in this script: SC2181:style:29:7: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint
[error] 320-320: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
.github/workflows/pull-request.yml (1)
298-337: Remove trailing spacesThere are trailing spaces on several lines that should be removed:
- Line 298: Empty line with spaces
- Line 302: After
steps:- Line 319: After
done- Line 337: After
number }}Remove trailing spaces to improve code cleanliness.
🧰 Tools
🪛 yamllint
[error] 298-298: trailing spaces
(trailing-spaces)
[error] 302-302: trailing spaces
(trailing-spaces)
[error] 319-319: trailing spaces
(trailing-spaces)
[error] 337-337: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/pull-request.yml
[error] 298-298: trailing spaces
(trailing-spaces)
[error] 302-302: trailing spaces
(trailing-spaces)
[error] 319-319: trailing spaces
(trailing-spaces)
[error] 337-337: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/pull-request.yml (1)
299-337: Verify Docker configuration and test data setup
Let's verify the Docker configuration and ensure test data setup is handled properly.
🧰 Tools
🪛 yamllint
[error] 302-302: trailing spaces
(trailing-spaces)
[error] 319-319: trailing spaces
(trailing-spaces)
[error] 337-337: trailing spaces
(trailing-spaces)
| Docker-Check: | ||
| needs: Test-Application | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
Add missing required steps
The job is missing critical setup steps:
- Repository checkout to access docker-compose files
- Node.js setup for environment consistency with other jobs
Add these steps before the Docker check:
Docker-Check:
needs: Test-Application
runs-on: ubuntu-latest
steps:
+ - name: Checkout repository
+ uses: actions/checkout@v4
+
+ - name: Set up Node.js
+ uses: actions/setup-node@v4
+ with:
+ node-version: '22.x'📝 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.
| Docker-Check: | |
| needs: Test-Application | |
| runs-on: ubuntu-latest | |
| steps: | |
| Docker-Check: | |
| needs: Test-Application | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| - name: Set up Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '22.x' |
🧰 Tools
🪛 yamllint
[error] 302-302: trailing spaces
(trailing-spaces)
| env: | ||
| HEALTH_CHECK_URL: http://localhost:4000 | ||
| COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
Add missing required environment variables
The Docker environment is missing critical variables that are required for the application to start properly. These variables are present in the Test-Application job but missing here.
Add these environment variables:
env:
HEALTH_CHECK_URL: http://localhost:4000
- COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}
+ COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}
+ MONGO_DB_URL: mongodb://mongodb:27017/talawa-test-db
+ REDIS_HOST: redis
+ REDIS_PORT: 6379
+ ACCESS_TOKEN_SECRET: ${{ env.ACCESS_TOKEN_SECRET }}
+ REFRESH_TOKEN_SECRET: ${{ env.REFRESH_TOKEN_SECRET }}
+ LAST_RESORT_SUPERADMIN_EMAIL: "abc@gmail.com"
+ COLORIZE_LOGS: "true"
+ LOG_LEVEL: "info"Note: The MongoDB and Redis hostnames are changed to match the service names in docker-compose.
📝 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.
| env: | |
| HEALTH_CHECK_URL: http://localhost:4000 | |
| COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }} | |
| env: | |
| HEALTH_CHECK_URL: http://localhost:4000 | |
| COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }} | |
| MONGO_DB_URL: mongodb://mongodb:27017/talawa-test-db | |
| REDIS_HOST: redis | |
| REDIS_PORT: 6379 | |
| ACCESS_TOKEN_SECRET: ${{ env.ACCESS_TOKEN_SECRET }} | |
| REFRESH_TOKEN_SECRET: ${{ env.REFRESH_TOKEN_SECRET }} | |
| LAST_RESORT_SUPERADMIN_EMAIL: "abc@gmail.com" | |
| COLORIZE_LOGS: "true" | |
| LOG_LEVEL: "info" |
🧰 Tools
🪛 yamllint
[error] 337-337: trailing spaces
(trailing-spaces)
| - name: Check if Talawa API starts in Docker | ||
| run: | | ||
| # Ensure no containers are running | ||
| docker-compose -f docker-compose.dev.yaml down -v || true | ||
|
|
||
| # Start containers | ||
| if ! docker-compose -f docker-compose.dev.yaml up -d --build; then | ||
| echo "Failed to start containers" | ||
| exit 1 | ||
| fi | ||
|
|
||
| timeout=60 | ||
| until curl -sf "${HEALTH_CHECK_URL:-http://localhost:4000}/health" 2>/dev/null || [ $timeout -eq 0 ]; do | ||
| echo "Waiting for API to start... ($timeout seconds remaining)" | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done | ||
|
|
||
| if [ $timeout -eq 0 ]; then | ||
| echo "Error: API failed to start within timeout" | ||
| docker-compose -f docker-compose.dev.yaml logs | ||
| docker-compose -f docker-compose.dev.yaml down -v | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "API started successfully" | ||
|
|
||
| # Cleanup | ||
| if ! docker-compose -f docker-compose.dev.yaml down -v; then | ||
| echo "Failed to cleanup containers" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Enhance Docker health check implementation
The current implementation has several issues:
- No verification that required services (MongoDB, Redis) are running
- Health check might fail due to container networking
- Missing proper error handling and logging
Replace the implementation with this more robust version:
- name: Check if Talawa API starts in Docker
run: |
# Ensure no containers are running
docker-compose -f docker-compose.dev.yaml down -v || true
# Start containers
if ! docker-compose -f docker-compose.dev.yaml up -d --build; then
echo "Failed to start containers"
docker-compose -f docker-compose.dev.yaml logs
exit 1
fi
# Wait for MongoDB and Redis to be ready
+ echo "Waiting for MongoDB..."
+ timeout=30
+ until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
+ sleep 1
+ ((timeout--))
+ done
+
+ echo "Waiting for Redis..."
+ timeout=30
+ until docker-compose -f docker-compose.dev.yaml exec -T redis redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
+ sleep 1
+ ((timeout--))
+ done
# Wait for API to be healthy
timeout=60
- until curl -sf "${HEALTH_CHECK_URL:-http://localhost:4000}/health" 2>/dev/null || [ $timeout -eq 0 ]; do
+ until docker-compose -f docker-compose.dev.yaml exec -T api curl -sf "http://localhost:4000/health" 2>/dev/null || [ $timeout -eq 0 ]; do
echo "Waiting for API to start... ($timeout seconds remaining)"
sleep 1
((timeout--))
done
if [ $timeout -eq 0 ]; then
echo "Error: API failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs
docker-compose -f docker-compose.dev.yaml down -v
exit 1
fi
echo "API started successfully"
# Cleanup
if ! docker-compose -f docker-compose.dev.yaml down -v; then
echo "Failed to cleanup containers"
exit 1
fi📝 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.
| - name: Check if Talawa API starts in Docker | |
| run: | | |
| # Ensure no containers are running | |
| docker-compose -f docker-compose.dev.yaml down -v || true | |
| # Start containers | |
| if ! docker-compose -f docker-compose.dev.yaml up -d --build; then | |
| echo "Failed to start containers" | |
| exit 1 | |
| fi | |
| timeout=60 | |
| until curl -sf "${HEALTH_CHECK_URL:-http://localhost:4000}/health" 2>/dev/null || [ $timeout -eq 0 ]; do | |
| echo "Waiting for API to start... ($timeout seconds remaining)" | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: API failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi | |
| echo "API started successfully" | |
| # Cleanup | |
| if ! docker-compose -f docker-compose.dev.yaml down -v; then | |
| echo "Failed to cleanup containers" | |
| exit 1 | |
| fi | |
| - name: Check if Talawa API starts in Docker | |
| run: | | |
| # Ensure no containers are running | |
| docker-compose -f docker-compose.dev.yaml down -v || true | |
| # Start containers | |
| if ! docker-compose -f docker-compose.dev.yaml up -d --build; then | |
| echo "Failed to start containers" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| exit 1 | |
| fi | |
| # Wait for MongoDB and Redis to be ready | |
| echo "Waiting for MongoDB..." | |
| timeout=30 | |
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| echo "Waiting for Redis..." | |
| timeout=30 | |
| until docker-compose -f docker-compose.dev.yaml exec -T redis redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| # Wait for API to be healthy | |
| timeout=60 | |
| until docker-compose -f docker-compose.dev.yaml exec -T api curl -sf "http://localhost:4000/health" 2>/dev/null || [ $timeout -eq 0 ]; do | |
| echo "Waiting for API to start... ($timeout seconds remaining)" | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: API failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi | |
| echo "API started successfully" | |
| # Cleanup | |
| if ! docker-compose -f docker-compose.dev.yaml down -v; then | |
| echo "Failed to cleanup containers" | |
| exit 1 | |
| fi |
🧰 Tools
🪛 yamllint
[error] 319-319: trailing spaces
(trailing-spaces)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2653 +/- ##
========================================
Coverage 97.74% 97.74%
========================================
Files 358 358
Lines 18114 18114
Branches 2600 2600
========================================
Hits 17706 17706
Misses 404 404
Partials 4 4 ☔ View full report in Codecov by Sentry. |
What kind of change does this PR introduce?
refactoring
2638
Fixes #2638
If relevant, did you update the documentation?
Summary
This pr ensures that the pr workflow checks whether the talawa api app starts in docker and the workflow passes
Have you read the contributing guide?
Summary by CodeRabbit
Docker-Check, to improve Docker container management during workflows.