Fix: Detachable Script Execution (Better Docs) #3317
Conversation
|
Warning Rate limit exceeded@JaiPannu-IITI has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 20 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 (1)
WalkthroughThe changes update the process for starting the Talawa API development server. The container configuration in Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Cont as Container (talawa-api-1)
Dev->>Cont: Initialize container (postCreate without auto-start)
Note right of Cont: Dependencies installed and environment set up
Dev->>Cont: Execute `docker exec talawa-api-1 /bin/bash -c 'pnpm run start_development_server'`
Cont-->>Dev: Development server starts
sequenceDiagram
participant Dev as Developer
participant Cont as Container
alt Attached mode
Dev->>Cont: Run interactive command to start server
Cont-->>Dev: Server logs are streamed back
else Detached mode
Dev->>Cont: Run `nohup` command to start server in background
Cont-->>Dev: Server starts silently
end
Possibly related PRs
Suggested labels
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
|
JaiPannu-IITI
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (3)
.devcontainer/devcontainer.json(1 hunks)docs/docs/docs/developer-resources/testing.md(1 hunks)docs/docs/docs/getting-started/installation.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/docs/docs/getting-started/installation.md
[grammar] ~310-~310: The word “output” is spelled as one word.
Context: ...tallation is complete, the last line of out put should look like this: ``` ... ... {"o...
(OUT_COMPOUNDS)
[grammar] ~342-~342: The word “shutdown” is a noun. The verb is spelled with a white space.
Context: ...and docker compose command to cleanly shutdown the dev container ``` docker compose d...
(NOUN_VERB_CONFUSION)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (3)
.devcontainer/devcontainer.json (1)
73-73: Deprecate Automatic Server Startup in Devcontainer
ThepostCreateCommandnow excludes thepnpm run start_development_servercommand, which is in line with the PR objectives. This change enables greater flexibility by requiring manual initiation of the development server, as detailed in the updated documentation.docs/docs/docs/developer-resources/testing.md (1)
820-823: Introduce Manual Development Server Startup
The addition of the command:docker exec talawa-api-1 /bin/bash -c 'pnpm run start_development_server'clearly guides developers on how to start the development server manually after resetting the Docker environment. This change compensates for the removed automatic startup from the devcontainer configuration and aligns well with the PR goals.
docs/docs/docs/getting-started/installation.md (1)
320-330: Attached Mode CLI Startup Instructions
The attached mode startup commands are clearly provided. The sequence—building the devcontainer, starting it up, and then executingdocker exec talawa-api-1 /bin/bash -c 'pnpm run start_development_server'ensures that the server starts in the foreground for active monitoring.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3317 +/- ##
=================================================
Coverage 48.57% 48.57%
=================================================
Files 458 458
Lines 34489 34489
Branches 971 971
=================================================
Hits 16752 16752
Misses 17737 17737 ☔ View full report in Codecov by Sentry. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
📜 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
310-310: Ordered list item prefix
Expected: 1; Actual: 12; Style: 1/1/1
(MD029, ol-prefix)
312-312: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
326-326: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
334-334: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
344-344: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ 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)
320-322: Updated CLI Startup Section.
The new “#### CLI Startup (Development)” header and the following description clearly outline how to start the dev container after installation. The instructions appear clear and align well with the PR objectives.
JaiPannu-IITI
changed the title
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
docs/docs/docs/getting-started/installation.md (4)
300-302: 🧹 Nitpick (assertive)Specify Language for JSON Output Code Block
The code block showing the expected output{"outcome":"success","imageName":"talawa-api"}lacks a language specifier. Adding a language identifier (e.g.
json) will improve readability and ensure compliance with markdown lint guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
300-300: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
327-330: 🧹 Nitpick (assertive)Add Language Identifier to Attached Mode Command Block
Please add a language specifier (e.g.bash) to the code block for the attached mode startup commands to enhance clarity and meet markdown lint recommendations.
334-338: 🧹 Nitpick (assertive)Add Language Identifier to Detached Mode Command Block
For the detached mode startup instructions, include a language identifier (e.g.bash) in the fenced code block to improve documentation quality and satisfy markdown lint guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
334-334: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
344-346: 🧹 Nitpick (assertive)Refine CLI Shutdown Section and Specify Language for Code Block
The shutdown command is presented as:docker compose downTo avoid redundancy and enhance clarity, consider rewording to “Use
docker compose downto cleanly shut down the dev container” and add a language specifier (e.g.bash) to the fenced code block.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
344-344: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/devcontainer.json(1 hunks)docs/docs/docs/developer-resources/testing.md(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
300-300: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
306-306: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
312-312: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
326-326: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
334-334: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
344-344: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (3)
.devcontainer/devcontainer.json (1)
73-73: Remove Automatic Server Startup from postCreateCommand
Removing thepnpm run start_development_servercommand from thepostCreateCommandnow grants developers explicit control over when to start the server—a change that aligns well with the PR objectives for a detachable script execution.docs/docs/docs/developer-resources/testing.md (1)
818-823: Add Manual Server Start Command for Docker Reset Instructions
The insertion ofdocker exec talawa-api-1 /bin/bash -c 'pnpm run start_development_server'at the end of the Docker reset instructions clearly provides developers with a manual way to initiate the development server after container restart. Please verify that the container name (
talawa-api-1) is consistent with your environment configuration.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
819-819: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
docs/docs/docs/getting-started/installation.md (1)
315-317: Confirm Consistent Expected Output Formatting
The expected JSON output for the container installation complete message is clearly specified. Please ensure that any future changes to the devcontainer build process maintain this output format so developers can reliably validate the installation.
|
@palisadoes PR is ready for review This explains why introducing one more step is more convenient.
|
palisadoes
merged commit 017a389
into
PalisadoesFoundation:develop-postgres
Overview
Depricating pnpm run start_development_server from postCreateCommand to gain complete control how server is start.
This allows us to start server inside docker and detach cli from it safely.
Related Issue: #3214
Recommended cron job:
cd /home/talawa-api/talawa-api && docker compose down --rmi all --volumes && docker compose down --rmi all --volumes && git checkout --force develop-postgres && git fetch upstream && git reset upstream/develop-postgres --hard && rm -rf node_modules && pnpm install && cp -f envFiles/.env.devcontainer .env && sed -i "s|MAPPED_PORT=80|MAPPED_PORT=8080|g" .env && sed -i "s|MAPPED_PORT=443|MAPPED_PORT=8443|g" .env && devcontainer build --workspace-folder . && devcontainer up --workspace-folder . && docker exec talawa-api-1 /bin/bash -c 'nohup pnpm run start_development_server > /dev/null 2>&1 &'For sample data it remains same
Note: This ensures successful execution provided cron has access to crud operation and deamon/pnpm, also docker service is running
Screenshots
This was run with above command , we can notice that in the end it does not struck to server listening but that in run inside docker in the back and detached safely
Summary by CodeRabbit
Chores
Documentation