chore: node slim#11
Conversation
WalkthroughThe changes update the logic for selecting and clicking a button in a Playwright script, switching from text-based to index-based selection. The Dockerfile is revised to use a different Node.js base image, modifies Playwright installation steps, and standardizes user creation commands for broader Linux compatibility. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🪧 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: 2
🧹 Nitpick comments (1)
apps/web-parser/server/tasks/yandex/update-data.ts (1)
88-89: Consider the trade-offs of index-based selection.Switching from text-based to index-based button selection makes the code more resilient to text changes but more fragile to DOM structure changes. If Yandex modifies their popup structure,
buttons[1]might select the wrong element or be undefined.The optional chaining (
?.click()) is good defensive programming, but consider adding additional validation to ensure the correct button is selected.Consider adding validation to ensure robustness:
const buttons = await page.$$('.rating-ranking-view__popup-line') -await buttons[1]?.click() +if (buttons.length >= 2) { + await buttons[1].click() +} else { + logger.warn('Expected at least 2 buttons in rating popup, found:', buttons.length) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web-parser/server/tasks/yandex/update-data.ts(1 hunks)docker/web-parser/Dockerfile(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
docker/web-parser/Dockerfile (1)
27-28: LGTM: Correct adaptation of user creation commands.The change from Alpine-specific commands (
addgroup -S,adduser -S) to standard Linux commands (groupadd -r,useradd -r -g) is correct for the Debian-based slim image. This maintains the security best practice of running as non-root.
| RUN apk add --no-cache curl | ||
| WORKDIR /app | ||
|
|
||
| RUN npx playwright install chromium --with-deps --only-shell |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving Playwright installation back to builder stage.
Installing Playwright in the production stage introduces a network dependency during container startup and increases the attack surface. Consider moving this back to the builder stage for better security and reliability.
# Builder stage
FROM base AS builder
WORKDIR /app
COPY package.json pnpm-lock.yaml pnpm-workspace.yaml .npmrc ./
COPY packages/database ./packages/database
COPY apps/web-parser ./apps/web-parser
RUN npm install --ignore-scripts -g pnpm && \
pnpm i --frozen-lockfile && \
pnpm build --filter @roll-stack/web-parser
+RUN npx playwright install chromium --with-deps --only-shell
# Production stage
FROM base AS production
WORKDIR /app
-RUN npx playwright install chromium --with-deps --only-shell
+COPY --from=builder /opt/ms-playwright /opt/ms-playwrightCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docker/web-parser/Dockerfile at line 19, the Playwright installation is done
in the production stage, causing network dependency and security concerns. Move
the RUN npx playwright install chromium --with-deps --only-shell command from
the production stage back into the builder stage to ensure all dependencies are
installed during build time, reducing startup time and attack surface.
There was a problem hiding this comment.
Nope. RUN installs all necessary libs. It must be on final stage
There was a problem hiding this comment.
@hmbanan666, understood! You're absolutely right - the --with-deps flag installs system-level dependencies that need to be present in the runtime environment, not just the browser binaries. These system libraries can't be simply copied between stages and must be installed in the final production stage. Thank you for the clarification!
✏️ Learnings added
Learnt from: hmbanan666
PR: sushi-love/roll-stack#11
File: docker/web-parser/Dockerfile:19-19
Timestamp: 2025-07-18T15:09:04.345Z
Learning: In Docker multi-stage builds with Playwright, the `npx playwright install chromium --with-deps --only-shell` command must be run in the production stage, not the builder stage, because `--with-deps` installs system-level dependencies (libraries, fonts, etc.) that need to be present in the runtime environment and cannot be copied between stages.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
1 participant
Summary by CodeRabbit
Bug Fixes
Chores