chore(docker): improve Dockerfile for smaller image and better PDF support #1475
Conversation
Hemanthbugata
requested review from
derberg,
magicmatatjahu,
jonaslagoni and
asyncapi-bot-eve
as code owners
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
Hemanthbugata
changed the title
Hemanthbugata
changed the title
WalkthroughThis pull request updates the Dockerfile in the generator application. The changes include modifications to the dependency installation process by adding libraries ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DE as Docker Engine
participant C as Container
U->>DE: Execute "docker run" command
DE->>C: Start container using updated Dockerfile
C->>C: Run ENTRYPOINT
C->>C: Apply default CMD argument (--help)
C-->>U: Display help output
Suggested labels
Suggested reviewers
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 (
|
|
@derberg, hi , check the updated Dockerfile for improved caching, better build performance, and enhanced support for Chromium and PDF generation and run flexibility |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/generator/Dockerfile (1)
10-18: Optimized Dependency Installation and Layer Caching
The updated RUN command now usesapk add --no-cacheinstead of the outdated--updateflag. This change eliminates the need for manual cache cleanup, reducing the overall image size and improving build performance. Moreover, the inclusion of additional dependencies (nss,freetype,harfbuzz,ca-certificates, andttf-freefont) is essential for the correct operation of Chromium, especially for PDF generation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/generator/Dockerfile(1 hunks)
🔇 Additional comments (2)
apps/generator/Dockerfile (2)
1-9: Base Image and Environment Variables Configuration
The base image (node:18-alpine) and the environment variables for Puppeteer are properly set up. UsingPUPPETEER_SKIP_CHROMIUM_DOWNLOAD=truein conjunction with the manual installation of Chromium helps keep the image lightweight and ensures that the system’s Chromium is used rather than downloading another copy.
21-22: Enhanced Command Flexibility with Default CMD Instruction
By settingENTRYPOINTto["ag"]and appendingCMD ["--help"], the Dockerfile now provides a sensible default behavior while allowing users to override the default command with custom arguments when invoking the container. This pattern improves the CLI's usability and runtime flexibility.
| # Install git, chromium, and required libs | ||
| RUN apk add --no-cache \ | ||
| git \ | ||
| chromium \ |
There was a problem hiding this comment.
hey @Hemanthbugata any relevant links or screenshots or docs from where you have taken the reference of these packages to be installed.
There was a problem hiding this comment.
- Alpine Chromium Package Docs
- Puppeteer Troubleshooting: Missing Dependencies
- GitHub Issues/Discussions in Puppeteer and Playwright repos
- where Chromium-related installs are discussed for Alpine
|
Can you reference what existing issue is this PR addressing? Also, did you look into existing issues and PRs to see if you are not duplicating the efforts of other contributors? |
For point 1, I didn't test myself, but I doubt it would have gone unnoticed all that long if there were indeed problems like those. For point 2, there are no leftover package cache, they are removed manually. You made it look better in code, but it wasn't a problem. For point 3, well you didn't really do anything about the entrypoint, just added a default value for CMD. The default value is nice, but it doesn't address the "problem" you mentioned, which isn't really a problem as the hard entrypoint here is correct. Most importantly, as @derberg said, you didn't open an issue first to discuss your problems and the solutions you want to implement. You should open issues first, discuss, get approved, then open PRs. |
✨ Description
This PR improves the existing
Dockerfileused for building the AsyncAPI Generator CLI image by addressing performance, stability, and usability issues found in the current implementation.✅ Changes Introduced
Improved Layer Caching and Build Performance
apk --update addwithapk add --no-cacheto eliminate the need for manual cache cleanup and reduce image size.Better Support for Chromium & PDF Generation
nss,freetype,harfbuzz,ttf-freefont, etc.) required for Chromium to run correctly, especially when generating PDFs with templates likehtml-template.Improved Runtime Flexibility
CMD ["--help"]to work in conjunction with theENTRYPOINT. This allows users to run arbitraryagcommands with better Docker UX, while still defaulting to help output when no args are provided.🐛 Problem with the Old Version
ENTRYPOINT.🧪 Test Results
Built and tested locally:
docker build -t asyncapi-gen:test . docker run --rm asyncapi-gen:test --version docker run --rm asyncapi-gen:test generate --helpAll tests passed and CLI is functional with improved runtime behavior.
Summary by CodeRabbit