-
Notifications
You must be signed in to change notification settings - Fork 38
feat(types): Add LimitsConfig field to ComponentProperties #670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(types): Add LimitsConfig field to ComponentProperties #670
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a spelling nit! LGTM once @thomastaylor312 's comments are addressed.
crates/wadm-types/src/lib.rs
Outdated
/// these values at runtime using `wasmcloud:secrets/store`. | ||
#[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
pub secrets: Vec<SecretProperty>, | ||
/// This Config holds the componet's metadata properties like memory limits and execution time limits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// This Config holds the componet's metadata properties like memory limits and execution time limits | |
/// This Config holds the component's metadata properties like memory limits and execution time limits |
Got it—I'll have those resolved ASAP 🫡 |
…ular per-component resource control Signed-off-by: Aditya <[email protected]>
8c9ae26
to
6c50ea6
Compare
id: option<string>, | ||
config: list<config-property>, | ||
secrets: list<secret-property>, | ||
limits: list<limits-config>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should bump the version of the package and add a since
annotation on these. @brooksmtownsend any preference on whether we do a minor bump or a patch bump? I think with wit, if you add something to a record
it is a breaking change, right?
@thomastaylor312 Is it alright to proceed further with these considerations or keep a u64 type and specifically instruct users on a set unit like megabytes and milliseconds |
Sorry @Aditya1404Sal this fell through the cracks. I think human readable/parsable formats are great so go ahead with that |
- Modify LimitsConfig to wadm-types for uni-config scenarios only - Update ScaleComponent command to include limits field - Add compute_limits function in scaler/convert.rs using parse-size crate for validation - Support human-readable formats like 1GB, 512MB, 30s for manifest configuration~ Signed-off-by: Aditya <[email protected]>
for granular per-component resource control
Feature or Problem
This PR aims to allow the
max_linear_memory
field to be configurable by the wadm.yaml, which will ultimately be used for setting per component resource limits.Related Issues
Memory Limits Per Component
Release Information
Consumer Impact
Testing
Unit Test(s)
Acceptance or Integration
Manual Verification