-
-
Notifications
You must be signed in to change notification settings - Fork 720
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
reverting workflow changes #2498
reverting workflow changes #2498
Conversation
WalkthroughThe pull request modifies the GitHub Actions workflow in Changes
Possibly related PRs
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
|
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
|
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: 3
🧹 Outside diff range and nitpick comments (1)
.github/workflows/pull-request.yml (1)
309-321
: Enhance health check robustnessThe health check scripts could be improved for better error handling and logging.
timeout=120 echo "Starting health check with ${timeout}s timeout" + # Log the start time for debugging + start_time=$(date '+%Y-%m-%d %H:%M:%S') + echo "Health check started at: ${start_time}" while ! nc -z localhost 4321 && [ $timeout -gt 0 ]; do sleep 1 timeout=$((timeout-1)) if [ $((timeout % 10)) -eq 0 ]; then echo "Still waiting for app to start... ${timeout}s remaining" + # Add more diagnostic information + echo "Checking port status..." + ss -tlnp | grep 4321 || echo "Port 4321 not found in listening state" fi done if [ $timeout -eq 0 ]; then + echo "Health check failed at: $(date '+%Y-%m-%d %H:%M:%S')" + echo "Time elapsed: $((120 - timeout)) seconds" echo "Timeout waiting for application to start" + # Collect diagnostic information + echo "System resource usage:" + top -b -n 1 | head -n 20 exit 1 fiAlso applies to: 334-346
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(8 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
283-283: unexpected key "Start-App-Without-Docker" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
284-284: key "name" is duplicated in "Graphql-Inspector" job. previously defined at line:258,col:5
(syntax-check)
285-285: key "runs-on" is duplicated in "Graphql-Inspector" job. previously defined at line:259,col:5
(syntax-check)
287-287: key "if" is duplicated in "Graphql-Inspector" job. previously defined at line:257,col:5
(syntax-check)
288-288: key "steps" is duplicated in "Graphql-Inspector" job. previously defined at line:260,col:5
(syntax-check)
381-381: shellcheck reported issue in this script: SC2086:info:3:35: Double quote to prevent globbing and word splitting
(shellcheck)
381-381: shellcheck reported issue in this script: SC2086:info:10:6: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 284-284: duplication of key "name" in mapping
(key-duplicates)
[error] 285-285: duplication of key "runs-on" in mapping
(key-duplicates)
[error] 287-287: duplication of key "if" in mapping
(key-duplicates)
[error] 288-288: duplication of key "steps" in mapping
(key-duplicates)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
Line range hint 98-132
: Good security improvement!
The addition of comprehensive sensitive file patterns (.env*
, *.pem
, *.key
, etc.) enhances security by preventing unauthorized modifications to credential files and secrets.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 133-133: trailing spaces
(trailing-spaces)
45-45
: Verify compatibility with tj-actions/changed-files v45
The action has been updated from v40 to v45 across multiple jobs. While updating dependencies is good practice, major version jumps may introduce breaking changes.
Also applies to: 95-95, 154-154, 207-207
✅ Verification successful
Update to tj-actions/changed-files v45 requires attention to breaking changes
The update from v40 to v45 includes a breaking change in v45.0.0 that affects how renamed files are handled when using GitHub's API with output_renamed_files_as_deleted_and_added
set to true
. Let's verify if this setting is used in the workflow.
Let's check the actual usage of tj-actions/changed-files in the workflow files to understand the configuration:
The update to tj-actions/changed-files v45 is safe to proceed
The workflow uses basic features of tj-actions/changed-files (only_changed and all_changed_files_count outputs) and doesn't use the output_renamed_files_as_deleted_and_added
setting that was affected by the breaking change in v45. The update should not impact the current functionality.
Additionally, the changes between v40 and v45 include several improvements and bug fixes, particularly for handling forked repository PRs, which could be beneficial for the workflow's reliability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the changelog for breaking changes between v40 and v45
gh api repos/tj-actions/changed-files/releases \
--jq '.[] | select(.tag_name | test("^v(4[0-5])")) | {version: .tag_name, body: .body}' \
| grep -i "breaking"
Length of output: 63206
Script:
#!/bin/bash
# Check if output_renamed_files_as_deleted_and_added is used in any workflow files
rg -l "output_renamed_files_as_deleted_and_added" .github/workflows/
Length of output: 68
Script:
#!/bin/bash
# Get the full context of how tj-actions/changed-files is used in workflows
rg -A 10 "uses: tj-actions/changed-files" .github/workflows/
Length of output: 3040
.github/workflows/pull-request.yml
Outdated
- name: Present and Upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}} | ||
uses: codecov/codecov-action@v4 | ||
with: | ||
token: ${{ secrets.CODECOV_TOKEN }} | ||
verbose: true | ||
fail_ci_if_error: false | ||
files: './coverage/lcov.info' | ||
name: '${{env.CODECOV_UNIQUE_NAME}}' | ||
fail_ci_if_error: false |
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.
Remove duplicate fail_ci_if_error
parameter
The fail_ci_if_error
parameter is specified twice in the Codecov action configuration.
- name: Present and Upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}}
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true
fail_ci_if_error: false
files: './coverage/lcov.info'
name: '${{env.CODECOV_UNIQUE_NAME}}'
- fail_ci_if_error: false
📝 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: Present and Upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}} | |
uses: codecov/codecov-action@v4 | |
with: | |
token: ${{ secrets.CODECOV_TOKEN }} | |
verbose: true | |
fail_ci_if_error: false | |
files: './coverage/lcov.info' | |
name: '${{env.CODECOV_UNIQUE_NAME}}' | |
fail_ci_if_error: false | |
- name: Present and Upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}} | |
uses: codecov/codecov-action@v4 | |
with: | |
token: ${{ secrets.CODECOV_TOKEN }} | |
verbose: true | |
fail_ci_if_error: false | |
files: './coverage/lcov.info' | |
name: '${{env.CODECOV_UNIQUE_NAME}}' |
🧰 Tools
🪛 actionlint (1.7.4)
248-248: key "fail_ci_if_error" is duplicated in "with" section. previously defined at line:245,col:11. note that this key is case insensitive
(syntax-check)
🪛 yamllint (1.35.1)
[error] 248-248: duplication of key "fail_ci_if_error" in mapping
(key-duplicates)
.github/workflows/pull-request.yml
Outdated
Start-App-Without-Docker: | ||
name: Check if Talawa Admin app starts (No Docker) | ||
runs-on: ubuntu-latest | ||
needs: [Code-Quality-Checks, Test-Application] | ||
if: github.actor != 'dependabot' | ||
steps: |
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.
Fix YAML structure for new jobs
The Start-App-Without-Docker
job is incorrectly indented and duplicates keys from the previous job.
- Start-App-Without-Docker:
+ start-app-without-docker:
name: Check if Talawa Admin app starts (No Docker)
runs-on: ubuntu-latest
needs: [Code-Quality-Checks, Test-Application]
if: github.actor != 'dependabot'
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.
Start-App-Without-Docker: | |
name: Check if Talawa Admin app starts (No Docker) | |
runs-on: ubuntu-latest | |
needs: [Code-Quality-Checks, Test-Application] | |
if: github.actor != 'dependabot' | |
steps: | |
start-app-without-docker: | |
name: Check if Talawa Admin app starts (No Docker) | |
runs-on: ubuntu-latest | |
needs: [Code-Quality-Checks, Test-Application] | |
if: github.actor != 'dependabot' | |
steps: |
🧰 Tools
🪛 actionlint (1.7.4)
283-283: unexpected key "Start-App-Without-Docker" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
284-284: key "name" is duplicated in "Graphql-Inspector" job. previously defined at line:258,col:5
(syntax-check)
285-285: key "runs-on" is duplicated in "Graphql-Inspector" job. previously defined at line:259,col:5
(syntax-check)
287-287: key "if" is duplicated in "Graphql-Inspector" job. previously defined at line:257,col:5
(syntax-check)
288-288: key "steps" is duplicated in "Graphql-Inspector" job. previously defined at line:260,col:5
(syntax-check)
🪛 yamllint (1.35.1)
[error] 284-284: duplication of key "name" in mapping
(key-duplicates)
[error] 285-285: duplication of key "runs-on" in mapping
(key-duplicates)
[error] 287-287: duplication of key "if" in mapping
(key-duplicates)
[error] 288-288: duplication of key "steps" in mapping
(key-duplicates)
run: | | ||
timeout="${HEALTH_CHECK_TIMEOUT:-120}" | ||
echo "Starting health check with ${timeout}s timeout" | ||
while ! nc -z localhost 4321 && [ $timeout -gt 0 ]; do | ||
sleep 1 | ||
timeout=$((timeout-1)) | ||
if [ $((timeout % 10)) -eq 0 ]; then | ||
echo "Still waiting for app to start... ${timeout}s remaining" | ||
fi | ||
done | ||
if [ $timeout -eq 0 ]; then | ||
echo "Timeout waiting for application to start" | ||
echo "Container logs:" | ||
docker logs talawa-admin-app-container | ||
exit 1 | ||
fi | ||
echo "Port check passed, verifying health endpoint..." | ||
- name: Stop Docker Container |
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.
Double quote variables in shell scripts
Shell variables should be properly quoted to prevent word splitting and globbing issues.
- timeout="${HEALTH_CHECK_TIMEOUT:-120}"
+ timeout="${HEALTH_CHECK_TIMEOUT:-120}"
echo "Starting health check with ${timeout}s timeout"
- while ! nc -z localhost 4321 && [ $timeout -gt 0 ]; do
+ while ! nc -z localhost 4321 && [ "${timeout}" -gt 0 ]; do
sleep 1
- timeout=$((timeout-1))
+ timeout=$((timeout-1))
- if [ $((timeout % 10)) -eq 0 ]; then
+ if [ $((timeout % 10)) -eq 0 ]; then
echo "Still waiting for app to start... ${timeout}s remaining"
fi
done
- if [ $timeout -eq 0 ]; then
+ if [ "${timeout}" -eq 0 ]; then
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
381-381: shellcheck reported issue in this script: SC2086:info:3:35: Double quote to prevent globbing and word splitting
(shellcheck)
381-381: shellcheck reported issue in this script: SC2086:info:10:6: Double quote to prevent globbing and word splitting
(shellcheck)
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: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/pull-request.yml (1)
98-98
: Consider adding more sensitive file patternsGood addition of sensitive file patterns. Consider including these additional common patterns:
*.pem *.key *.cert *.password *.secret *.credentials +*.pfx +*.p12 +*.keystore +*.jks +id_rsa* +*.gpgAlso applies to: 127-132
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(8 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
282-282: unexpected key "Start-App-Without-Docker" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
283-283: key "name" is duplicated in "Graphql-Inspector" job. previously defined at line:257,col:5
(syntax-check)
284-284: key "runs-on" is duplicated in "Graphql-Inspector" job. previously defined at line:258,col:5
(syntax-check)
286-286: key "if" is duplicated in "Graphql-Inspector" job. previously defined at line:256,col:5
(syntax-check)
287-287: key "steps" is duplicated in "Graphql-Inspector" job. previously defined at line:259,col:5
(syntax-check)
380-380: shellcheck reported issue in this script: SC2086:info:3:35: Double quote to prevent globbing and word splitting
(shellcheck)
380-380: shellcheck reported issue in this script: SC2086:info:10:6: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 283-283: duplication of key "name" in mapping
(key-duplicates)
[error] 284-284: duplication of key "runs-on" in mapping
(key-duplicates)
[error] 286-286: duplication of key "if" in mapping
(key-duplicates)
[error] 287-287: duplication of key "steps" in mapping
(key-duplicates)
🔇 Additional comments (3)
.github/workflows/pull-request.yml (3)
240-248
: Remove duplicate fail_ci_if_error
parameter
The fail_ci_if_error
parameter is specified twice in the Codecov action configuration.
282-282
:
Fix job key casing and indentation
The job key should be in lowercase with hyphens for consistency with other jobs.
- Start-App-Without-Docker:
+ start-app-without-docker:
Likely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
282-282: unexpected key "Start-App-Without-Docker" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
45-45
: Verify compatibility with tj-actions/changed-files v45
The action has been updated from v40 to v45 across multiple jobs. Major version updates may introduce breaking changes.
Also applies to: 95-95, 154-154, 207-207
✅ Verification successful
Breaking changes detected in tj-actions/changed-files v45 upgrade
The upgrade from v40 to v45 includes a breaking change that affects how renamed files are handled when using GitHub's API with output_renamed_files_as_deleted_and_added=true
. The previous filename will now be included in the list of deleted files.
However, based on the current usage in the workflow file:
- The action is used primarily for conditional execution based on
only_changed
- No usage of
output_renamed_files_as_deleted_and_added
parameter - No direct consumption of renamed, deleted, or added files outputs
Therefore, the breaking change does not impact the current workflow implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in tj-actions/changed-files between v40 and v45
# Fetch the changelog or release notes
gh api repos/tj-actions/changed-files/releases --jq '.[] | select(.tag_name | test("^v(4[0-5])\\..*$")) | .body' | grep -i "breaking"
Length of output: 223
Script:
#!/bin/bash
# Description: Get detailed release notes for versions v40 to v45 to understand the breaking changes
gh api repos/tj-actions/changed-files/releases --jq '.[] | select(.tag_name | test("^v(4[0-5])\\..*$")) | "Version: " + .tag_name + "\n" + .body + "\n"'
Length of output: 44778
Script:
#!/bin/bash
# Description: Check the current usage of tj-actions/changed-files in the workflow file
rg -U "uses: tj-actions/changed-files@.*?\n.*?(?:with:|$)" .github/workflows/pull-request.yml -A 10
Length of output: 1609
timeout=120 | ||
echo "Starting production health check with ${timeout}s timeout" | ||
while ! nc -z localhost 4173 && [ $timeout -gt 0 ]; do | ||
sleep 1 | ||
timeout=$((timeout-1)) | ||
if [ $((timeout % 10)) -eq 0 ]; then | ||
echo "Still waiting for production app to start... ${timeout}s remaining" | ||
fi | ||
done | ||
if [ $timeout -eq 0 ]; then | ||
echo "Timeout waiting for production application to start" | ||
exit 1 | ||
fi |
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.
🛠️ Refactor suggestion
Improve shell script safety and reusability
-
The health check logic is duplicated across multiple places.
-
Shell variables should be properly quoted to prevent word splitting.
-
Create a reusable health check script:
+ # .github/workflows/scripts/health_check.sh
+ #!/bin/bash
+
+ port="$1"
+ timeout="${2:-120}"
+
+ echo "Starting health check with ${timeout}s timeout"
+ while ! nc -z localhost "${port}" && [ "${timeout}" -gt 0 ]; do
+ sleep 1
+ timeout=$((timeout-1))
+ if [ $((timeout % 10)) -eq 0 ]; then
+ echo "Still waiting for app to start... ${timeout}s remaining"
+ fi
+ done
+
+ if [ "${timeout}" -eq 0 ]; then
+ echo "Timeout waiting for application to start"
+ exit 1
+ fi
+ echo "App started successfully on port ${port}"
- Update the jobs to use the script:
- timeout=120
- echo "Starting health check with ${timeout}s timeout"
- while ! nc -z localhost 4321 && [ $timeout -gt 0 ]; do
- sleep 1
- timeout=$((timeout-1))
- if [ $((timeout % 10)) -eq 0 ]; then
- echo "Still waiting for app to start... ${timeout}s remaining"
- fi
- done
- if [ $timeout -eq 0 ]; then
- echo "Timeout waiting for application to start"
- exit 1
- fi
+ chmod +x .github/workflows/scripts/health_check.sh
+ .github/workflows/scripts/health_check.sh 4321 120
Also applies to: 333-346, 381-395
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.
@coderabbitai all the workflows aren't getting trigerred, check pull-request.yml 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.
@prayanshchh, it seems that the workflows aren't getting triggered due to indentation issues in your pull-request.yml
file. The new job definitions for Start-App-Without-Docker
and Docker-Start-Check
need to be properly indented under the jobs:
section.
Here's how you can adjust the indentation:
jobs:
Code-Quality-Checks:
# existing job definitions
Graphql-Inspector:
# existing job definitions
Start-App-Without-Docker:
name: Check if Talawa Admin app starts (No Docker)
runs-on: ubuntu-latest
needs: [Code-Quality-Checks, Test-Application]
if: github.actor != 'dependabot'
steps:
# steps here...
Docker-Start-Check:
name: Check if Talawa Admin app starts in Docker
runs-on: ubuntu-latest
needs: [Code-Quality-Checks, Test-Application]
if: github.actor != 'dependabot'
steps:
# steps here...
Check-Target-Branch:
# existing job definitions
Ensure that all jobs are correctly indented under the jobs:
key. Incorrect indentation can prevent GitHub Actions from recognizing and triggering the workflows.
You might also want to validate your YAML file using a linter or an online YAML validator to catch any syntax errors.
Let me know if this helps resolve the issue.
@@ -272,6 +279,126 @@ jobs: | |||
|
|||
- name: Validate Documents | |||
run: graphql-inspector validate './src/GraphQl/**/*.ts' './talawa-api/schema.graphql' | |||
Start-App-Without-Docker: | |||
name: Check if Talawa Admin app starts (No Docker) |
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.
Incorrect indentation
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.
done
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(8 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
380-380: shellcheck reported issue in this script: SC2086:info:3:35: Double quote to prevent globbing and word splitting
(shellcheck)
380-380: shellcheck reported issue in this script: SC2086:info:10:6: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/pull-request.yml (3)
61-61
: LGTM! Correct environment variable reference
The environment variable reference has been properly fixed to use the correct output syntax from the tj-actions/changed-files action.
282-283
:
Fix job indentation
The Start-App-Without-Docker
job is incorrectly indented. This will prevent the workflow from running properly.
- Start-App-Without-Docker:
+ start-app-without-docker:
name: Check if Talawa Admin app starts (No Docker)
Likely invalid or redundant comment.
45-45
: Verify compatibility with tj-actions/changed-files v45
The action has been updated from v40 to v45 across multiple jobs. Major version updates may introduce breaking changes.
Also applies to: 95-95, 154-154, 207-207
timeout=120 | ||
echo "Starting production health check with ${timeout}s timeout" | ||
while ! nc -z localhost 4173 && [ $timeout -gt 0 ]; do | ||
sleep 1 | ||
timeout=$((timeout-1)) | ||
if [ $((timeout % 10)) -eq 0 ]; then | ||
echo "Still waiting for production app to start... ${timeout}s remaining" | ||
fi | ||
done | ||
if [ $timeout -eq 0 ]; then | ||
echo "Timeout waiting for production application to start" | ||
exit 1 | ||
fi |
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.
🛠️ Refactor suggestion
Improve shell script safety and reduce duplication
The health check logic is duplicated across multiple places and has shell script safety issues:
-
Shell variables should be properly quoted
-
Health check logic should be extracted into a reusable script
-
Create a reusable health check script:
+ # .github/workflows/scripts/health_check.sh
+ #!/bin/bash
+
+ port="$1"
+ timeout="${2:-120}"
+
+ echo "Starting health check with ${timeout}s timeout"
+ while ! nc -z localhost "${port}" && [ "${timeout}" -gt 0 ]; do
+ sleep 1
+ timeout=$((timeout-1))
+ if [ $((timeout % 10)) -eq 0 ]; then
+ echo "Still waiting for app to start... ${timeout}s remaining"
+ fi
+ done
+
+ if [ "${timeout}" -eq 0 ]; then
+ echo "Timeout waiting for application to start"
+ exit 1
+ fi
+ echo "App started successfully on port ${port}"
- Update the jobs to use the script:
- timeout=120
- echo "Starting health check with ${timeout}s timeout"
- while ! nc -z localhost 4321 && [ $timeout -gt 0 ]; do
- sleep 1
- timeout=$((timeout-1))
- if [ $((timeout % 10)) -eq 0 ]; then
- echo "Still waiting for app to start... ${timeout}s remaining"
- fi
- done
- if [ $timeout -eq 0 ]; then
- echo "Timeout waiting for application to start"
- exit 1
- fi
+ chmod +x .github/workflows/scripts/health_check.sh
+ .github/workflows/scripts/health_check.sh 4321 120
Also applies to: 333-346, 381-395
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2498 +/- ##
=====================================================
- Coverage 95.82% 83.78% -12.04%
=====================================================
Files 295 312 +17
Lines 7037 8117 +1080
Branches 1520 1776 +256
=====================================================
+ Hits 6743 6801 +58
- Misses 98 1177 +1079
+ Partials 196 139 -57 ☔ View full report in Codecov by Sentry. |
please check @palisadoes @varshith257 |
c80b44f
into
PalisadoesFoundation:develop-postgres
* reverting workflow changes * changes 1 * identation fix
What kind of change does this PR introduce?
reverting workflow changes
Issue Number:
Fixes #
Did you add tests for your changes?
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
Start-App-Without-Docker
andDocker-Start-Check
for enhanced app startup verification.