workflow which ensures that talawa Api app starts in docker #2759
Conversation
WalkthroughThis pull request introduces a new Changes
Possibly related 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 ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
366-372: Use environment variable for health check URLThe health check URL is hardcoded. Consider using the
HEALTH_CHECK_URLenvironment variable:-until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://talawa-api-dev:4000/health" 2>&1 || [ $timeout -eq 0 ]; do +until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "${HEALTH_CHECK_URL}/health" 2>&1 || [ $timeout -eq 0 ]; do🧰 Tools
🪛 actionlint (1.7.4)
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 366-366: trailing spaces
(trailing-spaces)
392-407: Fix environment variables indentationThe
env:section should be at the same level assteps::- 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 }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 407-407: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 407-407: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
|
making changes as suggested |
…ansh-talawa-api into api_fail_docker merge# Please enter a commit message to explain why this merge is necessary,
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
311-320: Optimize cache configurationConsider adding package-lock.json to the cache key for better cache hits.
- name: Cache Node.js dependencies uses: actions/cache@v4 with: path: | ~/.npm node_modules - key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }} + key: ${{ runner.os }}-docker-check-node-${{ hashFiles('package-lock.json') }} restore-keys: | - ${{ runner.os }}-docker-check- + ${{ runner.os }}-docker-check-node-
333-337: Improve error handling in container startupConsider capturing and displaying the exit code for better debugging.
-if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then +if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then + exit_code=$? echo "Failed to start containers" + echo "Exit code: $exit_code" docker-compose -f docker-compose.dev.yaml logs exit 1 fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 407-407: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
307-307: Fix action step configuration syntax
The Node.js setup step has incorrect syntax.
-- uses: actions/setup-node@v4
+ - uses: actions/setup-node@v4392-407: Verify environment variable configuration
Please verify that these environment variables match the ones in docker-compose.dev.yaml.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 407-407: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
393-407: LGTM: Environment variables are well configuredThe environment variables are properly set with appropriate use of secrets. Just remove trailing spaces at line 407.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}
299-407: Consider enhancing the Docker health check implementationSome suggestions to improve the implementation:
- Consider using Docker's built-in HEALTHCHECK instruction in Dockerfile instead of shell-based health checks
- Consider implementing exponential backoff for health checks instead of fixed intervals
- Consider adding container resource limits to prevent resource exhaustion
These changes would make the health checks more robust and maintainable.
🧰 Tools
🪛 actionlint (1.7.4)
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 408-408: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
299-320: LGTM: Job configuration follows best practices
The job configuration is well-structured with proper dependency management and caching strategy.
353-364:
Fix Redis health check syntax
There's a syntax error in the if condition:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; thenLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
321-337: Fix formatting and enhance error logging.
- Remove trailing spaces after
run :and in the if condition- Add container logs to the error message when docker-compose file is not found
- run : | + run: | # Ensure no containers are running docker-compose -f docker-compose.dev.yaml down -v || true # Verify docker-compose file exists - if [ ! -f "docker-compose.dev.yaml" ]; then + if [ ! -f "docker-compose.dev.yaml" ]; then echo "Error: docker-compose.dev.yaml not found" + docker-compose -f docker-compose.dev.yaml logs exit 1 fi🧰 Tools
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
366-382: Enhance API health check implementation.Consider adding additional health check parameters to ensure comprehensive API readiness:
- Add request timeout to curl command
- Check for specific health check response content
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf --max-time 5 "http://localhost:4000/health" 2>&1 | grep -q '"status":"healthy"' || [ $timeout -eq 0 ]; do🧰 Tools
🪛 yamllint (1.35.1)
[error] 366-366: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
384-384: could not parse as YAML: yaml: line 384: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 409-409: trailing spaces
(trailing-spaces)
[error] 388-388: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
299-320: LGTM! Job configuration follows best practices.
The job configuration is well-structured with proper dependency setup, Node.js configuration, and dependency caching.
299-409: Overall implementation achieves the PR objective.
The Docker check implementation successfully verifies that the Talawa API starts correctly in a Docker environment. It includes proper health checks, timeouts, and cleanup procedures. The remaining issues are mostly syntax and formatting related, which have been addressed in previous comments.
🧰 Tools
🪛 actionlint (1.7.4)
384-384: could not parse as YAML: yaml: line 384: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 409-409: trailing spaces
(trailing-spaces)
[error] 388-388: syntax error: could not find expected ':'
(syntax)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
367-380: Fix indentation in API health check.The indentation is inconsistent in this section, which affects readability.
- # Wait for TALAWA API to be healthy - until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do - echo "Waiting for API to start... ($timeout seconds remaining)" - sleep 1 - ((timeout--)) - done + # Wait for TALAWA API to be healthy + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do + echo "Waiting for API to start... ($timeout seconds remaining)" + sleep 1 + ((timeout--)) + done🧰 Tools
🪛 yamllint (1.35.1)
[error] 367-367: trailing spaces
(trailing-spaces)
394-409: Fix trailing spaces in environment variables section.Remove trailing spaces to maintain consistent formatting.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} +🧰 Tools
🪛 yamllint (1.35.1)
[warning] 395-395: wrong indentation: expected 6 but found 8
(indentation)
[error] 409-409: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 367-367: trailing spaces
(trailing-spaces)
[warning] 395-395: wrong indentation: expected 6 but found 8
(indentation)
[error] 409-409: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
299-320: LGTM! Job configuration follows best practices.
The job configuration is well-structured with proper dependency management, Node.js setup, and dependency caching.
321-338: LGTM! Docker startup implementation is robust.
The implementation includes proper error handling, timeouts, and cleanup of existing containers.
🧰 Tools
🪛 actionlint (1.7.4)
322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
343-343: Fix MongoDB health check syntax.
There's a syntax error in the until condition that needs to be fixed.
-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
+until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do360-365: Fix Redis health check syntax and indentation.
There are syntax errors in the Redis health check implementation.
-if[ $timeout -eq 0 ]; then
+if [ $timeout -eq 0 ]; then384-393: LGTM! Cleanup implementation follows best practices.
The cleanup function correctly preserves the exit code and uses trap for proper cleanup on script exit.
|
There isn’t an issue assigned to you for this PR. Please follow the guidelines in our PR_GUIDELINES.md file. We have the procedures in place so that everyone has a fair chance of contributing. I will be closing this pull request. Please follow the procedures and resubmit when ready. |
|
@Cioppolo14 I accidently tagged the wrong issue, but I am assigned to this issue: #2638..please re-open this |
|
Please make sure all tests pass and coderabbit.ai approves the PR |
yes sir, will get this ready 5-6 days |
|
Hi @prayanshchh . Have you found any solution for |
is that so, then ig this should be a seperate issue then, can u give ur PR link |
|
Sure. I have opened an issue regarding this since a few others also reported the same in slack. |
|
If you need any assistance feel free to reach out to me @prayanshchh @bandhan-majumder . |
|
Please fix:
Make sure CodeRabbit.ai approves your changes |
yes sir on it, I am first solving getVolunteerRanks implementation, the test is failing for a lot of pr's, if that is not fixed this won't can't be merged, once that is done, I will get back on this |
|
will it be okay if I create a new PR for this @palisadoes |
|
There is only one conflicting file. Please make your modifications and keep this PR open. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2759 +/- ##
========================================
Coverage 97.74% 97.74%
========================================
Files 358 358
Lines 18115 18115
Branches 2599 2599
========================================
Hits 17707 17707
Misses 404 404
Partials 4 4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/pull-request.yml (3)
321-325: Fix trailing spaces in the run command.Remove trailing spaces after the pipe operator.
- run: | + run: |
389-398: Fix indentation in cleanup function.The cleanup function implementation is good but needs formatting fixes:
- } + }
399-414: Fix environment variables indentation.The environment variables section has inconsistent indentation:
- HEALTH_CHECK_URL: http://localhost:4000 + HEALTH_CHECK_URL: http://localhost:4000 - +🧰 Tools
🪛 yamllint (1.35.1)
[warning] 400-400: wrong indentation: expected 6 but found 8
(indentation)
[error] 414-414: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
327-327: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 372-372: trailing spaces
(trailing-spaces)
[warning] 400-400: wrong indentation: expected 6 but found 8
(indentation)
[error] 414-414: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
299-320: LGTM! Job setup and caching configuration looks good.
The job configuration follows best practices with proper dependency management and caching strategy.
326-343: LGTM! Robust Docker container management implementation.
The implementation includes:
- Proper cleanup of existing containers
- File existence verification
- Timeout for container startup
- Error logging
🧰 Tools
🪛 actionlint (1.7.4)
327-327: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
344-388:
Fix syntax and indentation issues in health checks.
There are several syntax and indentation issues in the health checks:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; then
- # Wait for TALAWA API to be healthy
+ # Wait for TALAWA API to be healthy
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
+ until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; doLikely invalid or redundant comment.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 372-372: trailing spaces
(trailing-spaces)
399-414: Verify Docker configuration alignment.
Let's verify that the Docker configuration matches the environment variables:
✅ Verification successful
Docker configuration is properly aligned with environment variables
The verification confirms that the Docker configuration aligns correctly with the environment variables in the GitHub workflow:
- MongoDB configuration matches (
mongodb:27017) - Redis service name and port are correctly configured (
redis-stack-server:6379) - API port 4000 is properly exposed in both Dockerfile.dev and docker-compose.prod.yaml
- All required services (mongodb, redis-stack-server) are present with correct networking
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Docker configuration matches environment variables
# Check if docker-compose file exists and contains the required services
if [ -f "docker-compose.dev.yaml" ]; then
echo "Checking service configuration..."
# Verify MongoDB configuration
if ! grep -q "mongodb:27017" docker-compose.dev.yaml; then
echo "Warning: MongoDB port configuration mismatch"
fi
# Verify Redis configuration
if ! grep -q "redis-stack-server" docker-compose.dev.yaml; then
echo "Warning: Redis service name mismatch"
fi
# Verify API port configuration
if ! grep -q "4000" docker-compose.dev.yaml; then
echo "Warning: API port configuration missing"
fi
fi
Length of output: 342
Script:
#!/bin/bash
# Let's examine the docker-compose.dev.yaml content to better understand the configuration
cat docker-compose.dev.yaml
# Also check if there's a regular docker-compose.yaml
if [ -f "docker-compose.yaml" ]; then
echo -e "\n=== docker-compose.yaml content ===\n"
cat docker-compose.yaml
fi
# Let's also check for any other docker-compose files
fd docker-compose
Length of output: 1564
Script:
#!/bin/bash
# Let's check the production docker compose file as well
cat docker-compose.prod.yaml
# And let's verify if there are any port mappings in the Dockerfile.dev
cat Dockerfile.dev
Length of output: 1387
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 400-400: wrong indentation: expected 6 but found 8
(indentation)
[error] 414-414: trailing spaces
(trailing-spaces)
|
Please fix the failing test |
yes sir |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/pull-request.yml (2)
311-320: Enhance cache key strategyConsider adding
package-lock.jsonversion to the cache key for more precise caching:- key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }} + key: ${{ runner.os }}-docker-check-v1-${{ hashFiles('**/package-lock.json') }} restore-keys: | - ${{ runner.os }}-docker-check- + ${{ runner.os }}-docker-check-v1-
405-420: Fix trailing spaces in environment variables sectionRemove trailing spaces to maintain consistent formatting:
- HEALTH_CHECK_URL: http://localhost:4000 + HEALTH_CHECK_URL: http://localhost:4000 - MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}🧰 Tools
🪛 yamllint (1.35.1)
[error] 420-420: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
327-327: shellcheck reported issue in this script: SC1089:error:34:1: Parsing stopped here. Is this keyword correctly matched up?
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
395-404: LGTM! Well-implemented cleanup function
The cleanup function correctly preserves the exit code and uses trap for proper cleanup.
378-391:
Fix API health check indentation
The inconsistent indentation could cause YAML parsing errors:
- # Wait for TALAWA API to be healthy
+ # Wait for TALAWA API to be healthy
timeout=60
until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
echo "Waiting for API to start... ($timeout seconds remaining)"
sleep 1
((timeout--))
- done
+ done
- if [ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; thenLikely invalid or redundant comment.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
348-363:
Fix MongoDB health check syntax
There are several syntax issues in this section:
- 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
+ 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--))
+ ((timeout--))
done
- else
- echo "MongoDB is ready!"
- fi
+ echo "MongoDB is ready!"Likely invalid or redundant comment.
366-376:
Fix Redis health check syntax
There are several syntax issues in this section:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; then
echo "Error: Redis failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs redis-stack-server
docker-compose -f docker-compose.dev.yaml down -v
exit 1
- fi
+ fiLikely invalid or redundant comment.
| # Verify docker-compose file exists | ||
| if [ ! -f "docker-compose.dev.yaml" ]; then | ||
| echo "Error: docker-compose.dev.yaml not found" | ||
| docker-compose -f docker-compose.dev.yaml logs | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Start containers | ||
| if ! timeout 300 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 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for docker-compose file check
The current implementation logs docker-compose logs even when the file doesn't exist, which would fail.
if [ ! -f "docker-compose.dev.yaml" ]; then
echo "Error: docker-compose.dev.yaml not found"
- docker-compose -f docker-compose.dev.yaml logs
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.
| # Verify docker-compose file exists | |
| if [ ! -f "docker-compose.dev.yaml" ]; then | |
| echo "Error: docker-compose.dev.yaml not found" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| exit 1 | |
| fi | |
| # Start containers | |
| if ! timeout 300 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 | |
| # Verify docker-compose file exists | |
| if [ ! -f "docker-compose.dev.yaml" ]; then | |
| echo "Error: docker-compose.dev.yaml not found" | |
| exit 1 | |
| fi | |
| # Start containers | |
| if ! timeout 300 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/pull-request.yml (1)
331-336:⚠️ Potential issueFix docker-compose logs command placement
The docker-compose logs command will fail if the file doesn't exist.
Apply this fix:
if [ ! -f "docker-compose.dev.yaml" ]; then echo "Error: docker-compose.dev.yaml not found" - docker-compose -f docker-compose.dev.yaml logs exit 1 fi
🧹 Nitpick comments (3)
.github/workflows/pull-request.yml (3)
348-352: Fix indentation in MongoDB health check loopThe indentation is inconsistent in the MongoDB health check loop.
Apply this fix:
until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do -echo "Waiting for MongoDB to be ready..." -sleep 1 - ((timeout--)) + echo "Waiting for MongoDB to be ready..." + sleep 1 + ((timeout--)) done
380-384: Fix indentation in API health check loopThe indentation is inconsistent in the API health check loop.
Apply this fix:
until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do - echo "Waiting for API to start... ($timeout seconds remaining)" - sleep 1 - ((timeout--)) - done + echo "Waiting for API to start... ($timeout seconds remaining)" + sleep 1 + ((timeout--)) +done
299-420: Consider adding timeout for the entire jobWhile individual operations have timeouts, consider adding a timeout for the entire job to prevent long-running workflows.
Add the following at the job level:
Docker-Check: needs: Test-Application runs-on: ubuntu-latest + timeout-minutes: 15 steps:🧰 Tools
🪛 actionlint (1.7.4)
327-327: shellcheck reported issue in this script: SC1069:error:44:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
327-327: shellcheck reported issue in this script: SC1069:error:44:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
299-320: LGTM! Job configuration looks good.
The job configuration follows GitHub Actions best practices with proper dependency setup and caching.
396-404: LGTM! Cleanup function implementation is correct.
The cleanup function correctly preserves the exit code and ensures proper resource cleanup.
405-419: LGTM! Environment variables are well configured.
The environment variables are properly set with secure token generation using GitHub-provided secrets.
371-376:
Fix syntax in Redis health check condition
There's a syntax error in the Redis health check condition.
Apply this fix:
-if[ $timeout -eq 0 ]; then
+if [ $timeout -eq 0 ]; then
echo "Error: Redis failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs redis-stack-server
docker-compose -f docker-compose.dev.yaml down -v
exit 1
- fi
+fiLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
299-320: LGTM: Job configuration looks good!
The job configuration follows GitHub Actions best practices with proper dependency setup and caching strategy.
331-336: Remove unnecessary docker-compose logs command
The docker-compose logs command will fail since the file doesn't exist yet.
if [ ! -f "docker-compose.dev.yaml" ]; then
echo "Error: docker-compose.dev.yaml not found"
- docker-compose -f docker-compose.dev.yaml logs
exit 1
fi396-404: LGTM: Cleanup function implementation
The cleanup function correctly preserves the exit code and ensures proper resource cleanup.
405-419: LGTM: Environment variables configuration
The environment variables are well-configured with secure token generation using GitHub-specific variables.
380-391:
Fix API health check indentation
The API health check has inconsistent indentation.
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $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
+ until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $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
+ fiLikely invalid or redundant comment.
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do | ||
| echo "Waiting for MongoDB to be ready..." | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done |
There was a problem hiding this comment.
Fix MongoDB health check indentation
The indentation in the MongoDB health check loop is inconsistent.
until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do
-echo "Waiting for MongoDB to be ready..."
-sleep 1
- ((timeout--))
+ echo "Waiting for MongoDB to be ready..."
+ sleep 1
+ ((timeout--))
done📝 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.
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| echo "Waiting for MongoDB to be ready..." | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| echo "Waiting for MongoDB to be ready..." | |
| sleep 1 | |
| ((timeout--)) | |
| done |
| until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done | ||
| if [ $timeout -eq 0 ]; then | ||
| echo "Error: Redis failed to start within timeout" | ||
| docker-compose -f docker-compose.dev.yaml logs redis-stack-server | ||
| docker-compose -f docker-compose.dev.yaml down -v | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fix Redis health check indentation and spacing
The Redis health check has inconsistent indentation and spacing issues.
- timeout=30
- until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
- sleep 1
- ((timeout--))
- done
- if [ $timeout -eq 0 ]; then
- echo "Error: Redis failed to start within timeout"
- docker-compose -f docker-compose.dev.yaml logs redis-stack-server
- docker-compose -f docker-compose.dev.yaml down -v
- exit 1
- fi
+ timeout=30
+ until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
+ sleep 1
+ ((timeout--))
+ done
+ if [ $timeout -eq 0 ]; then
+ echo "Error: Redis failed to start within timeout"
+ docker-compose -f docker-compose.dev.yaml logs redis-stack-server
+ docker-compose -f docker-compose.dev.yaml down -v
+ 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.
| until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: Redis failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs redis-stack-server | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi | |
| timeout=30 | |
| until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: Redis failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs redis-stack-server | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi |
|
the docker workflow issue in this pr is related to this issue #2598 , this is the reason I wanted to fix the issue, as the issue is not assigned to anyone, would like to work on it, this won't be completed until and unless that issue is fixed. thoughts? @palisadoes |
Thanks. I've assigned the issue to you. Please fix both in this PR. |
yes sir, on it |
|
I have solved the redis-stack-server issue and pwd variable is set in setup.ts, but when containers are started i get this logs |
What kind of change does this PR introduce?
Changing workflow to ensure that talawa api starts in docker
Issue Number:
#2638
Fixes #
Did you add tests for your changes?
No
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Docker-Checkjob in the GitHub Actions workflow to enhance Docker setup and verification.Bug Fixes
Docker-Checkjob configuration.Tests