chore: some updates#10
Conversation
|
Warning Rate limit exceeded@hmbanan666 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 28 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 ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughThe Dockerfile for the web-parser service was updated to set the Playwright browser path environment variable earlier and streamline the copying of browser binaries between build stages. The health check script was also changed to query a different endpoint, updating the URL from Changes
Sequence Diagram(s)sequenceDiagram
participant HealthCheckScript as health-check.sh
participant WebParserService as web-parser (localhost:3000)
HealthCheckScript->>WebParserService: GET /api/health
WebParserService-->>HealthCheckScript: 200 OK or error
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
docker/web-parser/Dockerfile (1)
2-2: Environment variable placement looks good – consider also skipping browser re-downloads.Defining
PLAYWRIGHT_BROWSERS_PATHin the base stage is spot-on.
To shave ~40 MB and avoid any surprise network calls during laterpnpm i/nodeinvocations, it’s usually worth adding:ENV PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1right next to this line. The builder stage already ships the browsers, so forcing Playwright to skip the download is safe and speeds up CI/CD.
docker/web-parser/health-check.sh (1)
2-2: Harden the health-check curl invocation.Minor robustness tweaks:
-curl -f http://localhost:3000/api/health || exit 1 +curl --fail --silent --show-error --max-time 3 http://localhost:3000/api/health || exit 1•
--silent --show-errorremoves noise while still surfacing real issues.
•--max-time 3prevents the container from hanging if the service half-opens a socket.
• Behaviour otherwise identical (non-200 ⇒ exit 1).
Feel free to ignore if the current script already meets your SLOs.
| COPY --from=builder /root/.cache/ms-playwright /opt/ms-playwright | ||
| ENV PLAYWRIGHT_BROWSERS_PATH=/opt/ms-playwright | ||
|
|
||
| COPY --from=builder /opt/ms-playwright /opt/ms-playwright |
There was a problem hiding this comment.
🛠️ Refactor suggestion
COPY as non-root to prevent permission quirks.
All files under /opt/ms-playwright are currently owned by root.
Running the app as appuser works for read-only access, but Playwright occasionally writes temporary data (e.g., user-data-dir). A safer pattern is:
-COPY --from=builder /opt/ms-playwright /opt/ms-playwright
+COPY --chown=appuser:appgroup --from=builder /opt/ms-playwright /opt/ms-playwrightThat keeps the container 100 % read-/write consistent and avoids subtle “permission denied” crashes when upgrading Playwright in the future.
📝 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.
| COPY --from=builder /opt/ms-playwright /opt/ms-playwright | |
| COPY --chown=appuser:appgroup --from=builder /opt/ms-playwright /opt/ms-playwright |
🤖 Prompt for AI Agents
In docker/web-parser/Dockerfile at line 21, the COPY command copies files owned
by root, causing permission issues when the app runs as a non-root user. To fix
this, copy the files as the non-root user or adjust ownership after copying so
that /opt/ms-playwright and its contents are owned by the appuser. This ensures
consistent read/write permissions and prevents permission denied errors during
runtime.
|
1 participant
Summary by CodeRabbit