Refresh Docker image#3538
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR rebuilds the Docker image with a multi-stage slim base, adds a reusable Docker smoke-test action, wires smoke testing into a new Docker workflow and the publish workflow, and adds a Dependabot docker update entry. ChangesDocker Build and Smoke Test Pipeline
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PyPI
participant DockerBuild
participant SmokeAction
GitHubActions->>PyPI: fetch latest datamodel-code-generator version
PyPI-->>GitHubActions: version
GitHubActions->>DockerBuild: build image with VERSION build-arg
GitHubActions->>SmokeAction: run smoke test against built image
SmokeAction-->>GitHubActions: pass/fail result
Related PRs: None identified. Suggested labels: ci, docker, dependencies Suggested reviewers: koxudaxi 🐰 A slim image now takes the stage, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
📚 Docs Preview: https://pr-3538.datamodel-code-generator.pages.dev |
Merging this PR will improve performance by 46.18%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.9 s | 1.3 s | +51.68% |
| ⚡ | WallTime | test_perf_complex_refs |
1.9 s | 1.3 s | +48.75% |
| ⚡ | WallTime | test_perf_deep_nested |
5.1 s | 3.5 s | +47.92% |
| ⚡ | WallTime | test_perf_graphql_style_pydantic_v2 |
773.3 ms | 527.1 ms | +46.72% |
| ⚡ | WallTime | test_perf_all_options_enabled |
5.3 s | 3.6 s | +46.29% |
| ⚡ | WallTime | test_perf_duplicate_names |
1,014.8 ms | 695.1 ms | +45.99% |
| ⚡ | WallTime | test_perf_openapi_large |
2.9 s | 2 s | +44.93% |
| ⚡ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.5 s | 1.7 s | +44.91% |
| ⚡ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.9 s | 1.3 s | +44.85% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2 |
3.5 s | 2.4 s | +43.86% |
| ⚡ | WallTime | test_perf_multiple_files_input |
3.4 s | 2.4 s | +42.28% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing refresh-docker-image (73d9195) with main (95a12c0)
Footnotes
-
98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
![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 (3)
.github/workflows/docker.yaml (1)
45-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated build+smoke steps across workflows.
These steps (Docker Buildx setup,
docker/build-push-actionsmoke build, smoke-test action invocation) are duplicated almost verbatim in.github/workflows/publish.yaml(lines 82-93), differing only in theVERSIONsource. Consider extracting a reusable workflow (workflow_call) parameterized byversionto avoid drift between the two call sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker.yaml around lines 45 - 58, The Docker Buildx setup, smoke build, and smoke-test steps are duplicated in this workflow and the publish pipeline, so extract them into a reusable workflow invoked with workflow_call and a version input. Refactor the shared logic around the Buildx setup, docker/build-push-action, and smoke-docker-image action so both callers reuse the same implementation, with only the VERSION source passed in as a parameter.Dockerfile (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
ARG VERSIONhas no default.Building the image locally without
--build-arg VERSION=...yieldsdatamodel-code-generator[http]==(empty pin), which pip will reject. Consider documenting the required build-arg or providing a sensible default (e.g. latest) to avoid a confusing local build failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 3, The Dockerfile’s ARG VERSION is required but has no default, causing local builds to produce an empty datamodel-code-generator[http] pin. Update the Dockerfile around ARG VERSION to either provide a sensible default or make the requirement explicit in a way that prevents an empty pip dependency, and ensure the build path that uses VERSION is checked so it fails clearly or remains valid when build-arg VERSION is omitted..github/workflows/publish.yaml (1)
82-93: 🩺 Stability & Availability | 🔵 TrivialSmoke test doesn't cover the arm64 build that gets published.
The smoke image is built with
load: truefor the native (amd64) platform only, but the subsequentBuild and pushstep (lines 94-101) shipslinux/amd64,linux/arm64. QEMU is already configured (line 79), so a broken arm64 build would still pass this smoke gate and reach Docker Hub. Givendatamodel-code-generator[http]is largely pure-Python, the practical risk is limited, but worth a callout.Separately, the amd64 layers are built twice (once for the smoke tag, once again in the final multi-arch push) with no cache sharing between the two
build-push-actionsteps — consider addingcache-from/cache-to: type=ghato both steps to avoid the redundant rebuild.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yaml around lines 82 - 93, The smoke test only validates the native amd64 image, so it can miss arm64 breakages before the multi-arch publish in the Docker build workflow. Update the existing `docker/build-push-action` smoke step and the final `Build and push` step to build/validate both target platforms (or otherwise ensure the smoke gate exercises the same multi-arch output), and reuse build cache between the two `build-push-action` invocations with `cache-from`/`cache-to: type=gha` to avoid rebuilding the amd64 layers twice.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 14-23: The final runtime image is still running as root because
the Dockerfile never switches to a non-root user after setting up the runtime.
Update the final stage to create or reuse a dedicated unprivileged user and add
a USER directive before the ENTRYPOINT, keeping the runtime environment owned by
that user; make the change in the Dockerfile around the existing
ENV/COPY/ENTRYPOINT setup.
---
Nitpick comments:
In @.github/workflows/docker.yaml:
- Around line 45-58: The Docker Buildx setup, smoke build, and smoke-test steps
are duplicated in this workflow and the publish pipeline, so extract them into a
reusable workflow invoked with workflow_call and a version input. Refactor the
shared logic around the Buildx setup, docker/build-push-action, and
smoke-docker-image action so both callers reuse the same implementation, with
only the VERSION source passed in as a parameter.
In @.github/workflows/publish.yaml:
- Around line 82-93: The smoke test only validates the native amd64 image, so it
can miss arm64 breakages before the multi-arch publish in the Docker build
workflow. Update the existing `docker/build-push-action` smoke step and the
final `Build and push` step to build/validate both target platforms (or
otherwise ensure the smoke gate exercises the same multi-arch output), and reuse
build cache between the two `build-push-action` invocations with
`cache-from`/`cache-to: type=gha` to avoid rebuilding the amd64 layers twice.
In `@Dockerfile`:
- Line 3: The Dockerfile’s ARG VERSION is required but has no default, causing
local builds to produce an empty datamodel-code-generator[http] pin. Update the
Dockerfile around ARG VERSION to either provide a sensible default or make the
requirement explicit in a way that prevents an empty pip dependency, and ensure
the build path that uses VERSION is checked so it fails clearly or remains valid
when build-arg VERSION is omitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2450ae3-296e-4549-b17c-e27003280eda
📒 Files selected for processing (5)
.github/actions/smoke-docker-image/action.yml.github/dependabot.yaml.github/workflows/docker.yaml.github/workflows/publish.yamlDockerfile
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3538 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 160 161 +1
Lines 34344 34363 +19
Branches 4011 4011
=========================================
+ Hits 34344 34363 +19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Breaking Change AnalysisResult: No breaking changes detected Reasoning: PR #3538 only modifies CI/CD infrastructure (GitHub Actions workflows, a composite smoke-test action, dependabot config) and the Dockerfile. There are no changes to the Python package's API, CLI options, code generation output, Jinja2 templates, default behavior, supported Python versions, or error handling. The Dockerfile switches its base image from python:3.12-alpine to python:3.14.2-slim-bookworm, adds a multi-stage venv build, and runs as a non-root appuser, but the ENTRYPOINT (datamodel-codegen) and CLI behavior are unchanged. The VERSION build-arg requirement affects only maintainers building the image, not end users. None of these constitute breaking changes for datamodel-code-generator users. This analysis was performed by Claude Code Action |
|
🎉 Released in 0.67.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Summary by CodeRabbit