Skip to content

Add upper bounds to MachineResources fields#146

Merged
odesenfans merged 1 commit into
mainfrom
feat/bound-machine-resources
Apr 21, 2026
Merged

Add upper bounds to MachineResources fields#146
odesenfans merged 1 commit into
mainfrom
feat/bound-machine-resources

Conversation

@odesenfans
Copy link
Copy Markdown
Contributor

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)

Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@odesenfans odesenfans force-pushed the feat/bound-machine-resources branch from edd3b0c to bae9b1d Compare April 21, 2026 21:44
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@odesenfans odesenfans merged commit 0386898 into main Apr 21, 2026
10 checks passed
@odesenfans odesenfans deleted the feat/bound-machine-resources branch April 21, 2026 21:55
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.

2 participants