Add upper bounds to MachineResources fields#146
Conversation
foxpatch-aleph
left a comment
There was a problem hiding this comment.
This PR correctly adds upper bounds to MachineResources fields using Pydantic Field constraints. The constants (256 vCPUs, 1 TiB memory, ~10 years) are reasonable and well-documented. Default values fall within valid ranges, and the implementation follows existing patterns. No new tests are needed as Pydantic's built-in validation handles the bounds checking automatically.
aleph_message/models/execution/environment.py (line 65): Minor nit: MAX_SECONDS uses 365 days/year (ignoring leap years), making it ~9.98 years rather than exactly 10. This is actually fine since it's slightly conservative, but you might consider using datetime.timedelta(days=3650).seconds for clarity if exactness matters.
Cap vcpus at 256, memory at 1 TiB (in MiB) and seconds at ~10 years. Keeps the values within realistic ranges and prevents overflow in downstream cost calculations.
edd3b0c to
bae9b1d
Compare
foxpatch-aleph
left a comment
There was a problem hiding this comment.
This PR correctly adds upper bounds to MachineResources fields to prevent overflow in downstream cost calculations. The constants are well-named, reasonably chosen (256 vCPUs, 1 TiB memory, ~10 years), and properly applied via Pydantic Field constraints. The code is clean, follows existing style, and maintains backward compatibility with unchanged defaults.
aleph_message/models/execution/environment.py (line 65): Minor suggestion: Consider using datetime.timedelta(days=3650).seconds or a constant like SECONDS_PER_YEAR = 365 * 24 * 60 * 60 for readability, but the current explicit calculation is also clear.
Cap vcpus at 256, memory at 1 TiB (in MiB) and seconds at ~10 years. Keeps the values within realistic ranges and prevents overflow in downstream cost calculations.
Test plan
hatch -e testing run pytest(unchanged — same 4 pre-existing network-flaky tests fail on main)