Warn user when overriding requests but not limits #2151
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are the changes needed?
As a user, I would expect that the resulting task pod has 5 vCPUs and 5Gi or RAM. Turns out, the task pod has 3 each!
This behaviour is very unintuitive, the reason being as follows:
What changes were proposed in this pull request?
In this PR I add a warning so that at least users are warned to set limit as well.
How was this patch tested?
Running the above workflow with
pyflyte -v run --remote ...
results in the following new log line:Requests overridden on node n0 (<FlyteLiteral name: "foo" retries { }>) without specifying limits. Requests are clamped to original limits.
The fact that
-v
is required to see this warning unfortunately limits its usefulness. Maybe this warrants a discussion whether this behaviour should be changed.