Remove ~25 MB of unnecessary packages from Docker image#100
Remove ~25 MB of unnecessary packages from Docker image#100nitrobass24 merged 1 commit intodevelopfrom
Conversation
|
Warning Rate limit exceeded
⌛ 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 selected for processing (9)
📝 WalkthroughWalkthroughReplaced runtime packages and privilege helper in Docker, removed pytz from Python deps and migrated tests to datetime.timezone, and introduced an internal boolean parser replacing distutils.util.strtobool with corresponding test updates for empty-string handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/docker/build/docker-image/Dockerfile`:
- Around line 59-66: The RUN step in the Dockerfile currently always runs ln -s
/usr/bin/7zz /usr/bin/7z which will fail if /usr/bin/7z already exists; update
that part of the RUN command (the same line that currently ends with ln -s
/usr/bin/7zz /usr/bin/7z) to be idempotent by checking for the target first,
e.g. replace the plain ln call with a conditional like [ -e /usr/bin/7z ] || ln
-s /usr/bin/7zz /usr/bin/7z (or use ln -sf if you prefer to force overwrite), so
the build won't fail when /usr/bin/7z is already present.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/docker/build/docker-image/Dockerfilesrc/docker/build/docker-image/entrypoint.shsrc/python/pyproject.tomlsrc/python/requirements.txtsrc/python/tests/unittests/test_web/test_serialize/test_serialize_model.pysrc/python/tests/unittests/test_web/test_serialize/test_serialize_status.py
💤 Files with no reviewable changes (2)
- src/python/pyproject.toml
- src/python/requirements.txt
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/python/common/config.py`:
- Around line 15-23: In _strtobool, replace the old %-formatting in the
exception with an f-string for consistency with modern Python; specifically
change the raise ValueError("invalid truth value %r" % (val,)) to use f-string
formatting such as raise ValueError(f"invalid truth value {val!r}") so the
message remains identical but follows current style.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/python/common/config.pysrc/python/tests/unittests/test_common/test_config.py
| def _strtobool(val: str) -> bool: | ||
| val = val.strip().lower() | ||
| if val in ('y', 'yes', 't', 'true', 'on', '1'): | ||
| return True | ||
| elif val in ('n', 'no', 'f', 'false', 'off', '0'): | ||
| return False | ||
| else: | ||
| raise ValueError("invalid truth value %r" % (val,)) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good replacement for distutils.util.strtobool.
The implementation correctly handles the same boolean tokens as the original distutils.util.strtobool, which is necessary since setuptools/distutils are being removed from the Docker image.
One minor style improvement flagged by static analysis: consider using f-string formatting instead of % formatting for consistency with modern Python.
✨ Optional: Use f-string formatting
- raise ValueError("invalid truth value %r" % (val,))
+ raise ValueError(f"invalid truth value {val!r}")🧰 Tools
🪛 Ruff (0.15.2)
[warning] 22-22: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/python/common/config.py` around lines 15 - 23, In _strtobool, replace the
old %-formatting in the exception with an f-string for consistency with modern
Python; specifically change the raise ValueError("invalid truth value %r" %
(val,)) to use f-string formatting such as raise ValueError(f"invalid truth
value {val!r}") so the message remains identical but follows current style.
- Replace p7zip + p7zip-full (5.4 MB) with 7zip (2.5 MB) - Replace gosu (2.2 MB) with setpriv (already in util-linux) - Remove pytz (2.8 MB), use stdlib datetime.timezone.utc instead - Remove pip + setuptools (~16 MB) after dependency installation - Replace distutils.util.strtobool with inline implementation - Fix 6 pre-existing config test failures (round-trip guard behavior) - Install lftp and openssh-client in test container for unit tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nitrobass24
force-pushed
the
chore/slim-docker-image
branch
from
d89b3c2 to
db4287d
Compare
1 participant
Summary
7zippackage provides7zz, which patoolib 4.x supports natively. Added7z → 7zzsymlink for compatibility.setprivis already available viautil-linux(a base package).datetime.timezone.utc— saves ~2.8 MB. Only 2 test files imported pytz; no runtime code uses it.Estimated total savings: ~24 MB (184.7 MB → ~160 MB)
Test plan
make build— Docker image builds successfullymake run— container starts with correct UID/GID handling (validates setpriv)7zz --helpand7z --help(symlink) workunrarworkswhich gosureturns nothing,which pipreturns nothingdocker image inspect --format '{{.Size}}'shows ~160 MB🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Dependencies
Bug Fixes
Tests