-
Notifications
You must be signed in to change notification settings - Fork 1
🐋 Use artillery docker image #10
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
Conversation
WalkthroughThe Dockerfile in the performance directory has been updated to simplify its configuration. The base image is now set to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DockerBuild
participant Container
User->>DockerBuild: Initiate build using Dockerfile
DockerBuild->>Container: Pull base image (artilleryio/artillery:2.0.21)
Container->>Container: Skip proxy and npm installation steps
Container->>Container: Execute CMD to run Artillery
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
CodeRabbit Configuration 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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
performance/Dockerfile (1)
17-17: 💡 Verification agent🧩 Analysis chain
CMD Instruction Format: Ensure Correct Command Invocation
The CMD is defined as:CMD ["run -e development", "-o reports/report.json", "scenario.yml"]In exec form each argument is treated as a literal string. Confirm that the intended command is executed properly. If the underlying image does not set an ENTRYPOINT to
artillery, you may need to explicitly include it. For example, consider splitting the command into its constituent parts as shown below:- CMD ["run -e development", "-o reports/report.json", "scenario.yml"] + CMD ["artillery", "run", "-e", "development", "-o", "reports/report.json", "scenario.yml"]Review this change against the base image’s defaults to ensure the container starts as expected.
🏁 Script executed:
#!/bin/bash echo "=== FROM directive in performance/Dockerfile ===" rg "^FROM" performance/Dockerfile echo "" echo "=== ENTRYPOINT directive in performance/Dockerfile ===" rg "^ENTRYPOINT" performance/Dockerfile echo "" echo "=== References to 'artillery' in performance/Dockerfile ===" rg "artillery" performance/DockerfileLength of output: 612
CMD Instruction Format Issue – Update Required
The current CMD in the Dockerfile:
CMD ["run -e development", "-o reports/report.json", "scenario.yml"]passes
"run -e development"as a single literal argument. In exec form, each argument should be a separate array element. Given that the underlying base image (artilleryio/artillery:2.0.21) may rely on the CMD to supply arguments to its ENTRYPOINT, it’s important to verify that the intended command is executed properly. If the base image does not set an ENTRYPOINT toartillery, or if you want to avoid ambiguity, explicitly including the command name in CMD is recommended. For example:- CMD ["run -e development", "-o reports/report.json", "scenario.yml"] + CMD ["artillery", "run", "-e", "development", "-o", "reports/report.json", "scenario.yml"]
- Ensure that the base image’s default ENTRYPOINT matches your intended usage.
- Verify that splitting the command into separate tokens does not alter the container’s behavior.
Please review and update the Dockerfile accordingly.
🧹 Nitpick comments (2)
performance/Dockerfile (2)
4-4: Base Image Update: Pinning Artillery Version
The base image is now set toartilleryio/artillery:2.0.21, which meets the PR objective of using a pre-configured image with Playwright and the requiredreportcommand.
Consider adding an inline comment explaining the choice (i.e. why version 2.0.21 is pinned) for future maintainers.
5-8: ARG Declarations for Proxy and Registry
The Dockerfile still declares build arguments forhttp_proxy,https_proxy,no_proxy, andnpm_registry. Since the PR removes the proxy configuration steps and npm registry setup, verify whether these ARGs are still needed. If not, it would simplify the Dockerfile to remove them.
reportpour générer le fichier html.Summary by CodeRabbit