updated sample_data import script, docs updated, githhub action to test #3146
Conversation
|
Warning Rate limit exceeded@gautam-divyanshu has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis PR introduces a new job named Changes
Assessment against linked issues
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/pull-request.yml(1 hunks)package.json(1 hunks)src/utilities/loadSampleData.ts(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/pull-request.yml
[error] 326-326: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (4)
src/utilities/loadSampleData.ts (2)
1-2: LGTM! Environment variables configuration added.The dotenv configuration is correctly implemented at the start of the file.
152-156: LGTM! Added data integrity checks.The error checks ensure that users and organizations exist before inserting posts and events, preventing potential data integrity issues.
Also applies to: 180-184
package.json (1)
20-20: Verify dotenv package version.The current version (^16.0.3) might not be the latest. Let's check for any updates or security advisories.
✅ Verification successful
🌐 Web query:
What is the latest version of dotenv npm package?Length of output: 2215
🏁 Script executed:
#!/bin/bash # This command queries the npm registry for the latest published version of the "dotenv" package. npm view dotenv versionLength of output: 32
dotenv version update verification
The latest version of the dotenv npm package is 16.4.7. Your package.json specifies "^16.0.3", which allows minor/patch upgrades within the major version 16. Consequently, a fresh installation should resolve to 16.4.7 unless the lock file prevents it.
.github/workflows/pull-request.yml (1)
315-326: LGTM! Sample data import test job added.The job configuration is well-structured and follows the project's patterns for Docker-based testing.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 326-326: no new line character at the end of file
(new-line-at-end-of-file)
|
@palisadoes most of the action are failing because of that corepack issue |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/docs/docs/getting-started/installation.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
297-297: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
301-301: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
303-303: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
303-303: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
304-304: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/docs/docs/getting-started/installation.md (1)
298-299: 🧹 Nitpick (assertive)Ensure Proper Markdown Heading Spacing
The newly added "#### Import Sample Data" heading should be surrounded by blank lines (both before and after) to comply with markdown lint guidelines (MD022) and improve overall readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/docs/docs/getting-started/installation.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
302-302: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
304-304: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
304-304: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
305-305: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (1)
docs/docs/docs/getting-started/installation.md (1)
302-306: 🧹 Nitpick (assertive)Close Fenced Code Blocks and Add Surrounding Blank Lines
The fenced code blocks for the shell commands in steps 2 and 3 are missing their closing triple backticks. Additionally, per MD031, these code blocks should be surrounded by blank lines. Please add a closing fence after the command in each block. For example:
@@ -303,0 + ```and
@@ -306,0 + ```This will ensure proper formatting of the shell command examples.
Likely invalid or redundant comment.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
302-302: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
304-304: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
304-304: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
305-305: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3146 +/- ##
====================================================
- Coverage 39.72% 39.71% -0.02%
====================================================
Files 455 455
Lines 33541 33553 +12
Branches 407 407
====================================================
Hits 13324 13324
- Misses 20217 20229 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/docs/docs/getting-started/installation.md (1)
298-298: 🧹 Nitpick (assertive)Markdown Heading Formatting
The heading "#### Import Sample Data" should be preceded (and ideally followed) by a blank line for better readability and to comply with markdown lint guidelines (MD022). Please insert a blank line before this heading.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pull-request.yml(1 hunks)docs/docs/docs/getting-started/installation.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
305-305: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
306-306: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
🪛 YAMLlint (1.35.1)
.github/workflows/pull-request.yml
[error] 332-332: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
|
Could it be one of these solutions? It may not be
|
|
yes, it is related to docker |
|
None of the other GitHub actions use the Dev container env file. Use the ci env file |
|
But when I use Docker Compose, it will still be unhealthy. |
|
same error using reverted pr, |
|
I think it was working fine earlier. |
|
I am getting this error with upstream also; can you also try? |
|
why is it skipping? |
|
I think it's related to the API running health checks on all the other processes but not running it on itself. There is an explanation from @xoldd in the #talawa-apin slack channel Is it hanging at that test? If it's not, then it's OK |
|
Ok, this is how it's supposed to be. |
|
This one requires a This one uses a GitHub action which probably does the same thing |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
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] 326-326: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Deployment to https://docs-api.talawa.io
🔇 Additional comments (1)
.github/workflows/pull-request.yml (1)
326-326: Verify Container Name Consistency
Double-check that the container nametalawa-api-1in thedocker execcommand matches the name defined by your Docker Compose configuration. Mismatched names might cause the command to fail unexpectedly.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 326-326: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/pull-request.yml
Outdated
| - name: Build talawa api non production environment docker image | ||
| run: docker compose build | ||
| - name: Import sample data | ||
| run: docker exec talawa-api-1 /bin/bash -c "pnpm run import:sample-data" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure Newline at End of File
A newline character is missing at the end of the file. Adding a newline will comply with YAML best practices and avoid linting errors.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 326-326: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/pull-request.yml
Outdated
| Import-Sample-Data: | ||
| name: Import Sample Data | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout this repository | ||
| uses: actions/checkout@v4.2.2 | ||
| - name: Create .env file for talawa api testing environment | ||
| run: cp ./envFiles/.env.ci ./.env | ||
| - name: Build talawa api non production environment docker image | ||
| run: docker compose build | ||
| - name: Import sample data | ||
| run: docker exec talawa-api-1 /bin/bash -c "pnpm run import:sample-data" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Container Startup Step Missing in "Import-Sample-Data" Job
Currently, the "Import-Sample-Data" job only runs the command docker compose build but does not start the container before executing the sample data import via docker exec. If the container is not running when the docker exec command is issued, the job will likely fail. Consider adding a step to start the container (e.g., using docker compose up -d) right after building the image.
Proposed diff:
- - name: Build talawa api non production environment docker image
- run: docker compose build
+ - name: Build and start talawa api container
+ run: |
+ docker compose build
+ docker compose up -d📝 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.
| Import-Sample-Data: | |
| name: Import Sample Data | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout this repository | |
| uses: actions/checkout@v4.2.2 | |
| - name: Create .env file for talawa api testing environment | |
| run: cp ./envFiles/.env.ci ./.env | |
| - name: Build talawa api non production environment docker image | |
| run: docker compose build | |
| - name: Import sample data | |
| run: docker exec talawa-api-1 /bin/bash -c "pnpm run import:sample-data" | |
| Import-Sample-Data: | |
| name: Import Sample Data | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout this repository | |
| uses: actions/checkout@v4.2.2 | |
| - name: Create .env file for talawa api testing environment | |
| run: cp ./envFiles/.env.ci ./.env | |
| - name: Build and start talawa api container | |
| run: | | |
| docker compose build | |
| docker compose up -d | |
| - name: Import sample data | |
| run: docker exec talawa-api-1 /bin/bash -c "pnpm run import:sample-data" |
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 326-326: no new line character at the end of file
(new-line-at-end-of-file)
Did you see this? |
yes, pushing the commits |
|
Here's the official documentation |
|
The job ID would be |
|
Can we merge this without GitHub action so that other guys can work? I will create a new PR for GitHub action. |
|
Comment out the GitHub action. |
palisadoes
merged commit 8dca2ac
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Issue Number:
Fixes #2841
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Chores
Bug Fixes