Skip to content

fix: accept all Docker-style memory limits in convert_mem_limit_to_bytes#14

Open
mkzung wants to merge 1 commit into
NethermindEth:mainfrom
mkzung:fix/mem-limit-parsing
Open

fix: accept all Docker-style memory limits in convert_mem_limit_to_bytes#14
mkzung wants to merge 1 commit into
NethermindEth:mainfrom
mkzung:fix/mem-limit-parsing

Conversation

@mkzung

@mkzung mkzung commented Jul 2, 2026

Copy link
Copy Markdown

Problem. convert_mem_limit_to_bytes only handled lowercase single-letter units and used str.replace, so valid Docker memory limits raised ValueError: 8G, 512M, 1gb, or a bare byte count all failed. The same mem_limit string is passed straight to Docker for the container limit in compressor.py (docker-py's parse_bytes accepts all of those), so a user could set --mem-limit 8G, Docker would take it for the container, and the memory-hint conversion would crash on the same value.

Fix. Parse it the way Docker does (case-insensitive, 1024-based): a number with an optional b/k/m/g unit (also written kb/mb/gb) or a bare byte count; clear ValueError on invalid input (empty, negative, non-finite, or unrecognised unit).

Tests / CI. CLAUDE.md already documents a pytest / tests/ workflow, but the repo had no tests/ directory and pytest was not in the dev group, so the documented pytest command did not work out of the box. This adds the first test module for the parser (valid + invalid cases, and a check that it agrees with docker-py's parse_bytes on configurable values), appends pytest to the existing dev group, and a small GitHub Actions workflow that runs the documented command on PRs (uv sync --dev + uv run pytest).

If a reviewer prefers, the CI workflow can be split into a follow-up PR.

@mkzung mkzung marked this pull request as ready for review July 2, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant