-
Notifications
You must be signed in to change notification settings - Fork 300
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
Make MatchableResources configurable through flyte-cli #118
Conversation
I still don't like the name MatchableResource |
Codecov Report
@@ Coverage Diff @@
## master #118 +/- ##
==========================================
- Coverage 81.60% 81.52% -0.08%
==========================================
Files 211 213 +2
Lines 13745 13878 +133
Branches 1133 1140 +7
==========================================
+ Hits 11216 11314 +98
- Misses 2262 2295 +33
- Partials 267 269 +2
Continue to review full report at Codecov.
|
flytekit/clis/flyte_cli/main.py
Outdated
|
||
Use a -- to separate arguments to this cli, and attributes used for cluster resource configuration. | ||
e.g. | ||
$ flyte-cli -h localhost:30081 -p flyteexamples -d development update-cluster-resource-attributes -- critical \ |
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.
It seems this would be better with like a YAML?
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.
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.
updated to use multi value options, thanks for the suggestions!
friendly ping |
flytekit/clis/flyte_cli/main.py
Outdated
|
||
Use a -- to separate arguments to this cli, and attributes used for cluster resource configuration. | ||
e.g. | ||
$ flyte-cli -h localhost:30081 -p flyteexamples -d development update-cluster-resource-attributes \ |
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.
copy paste?
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.
yep thanks for catching
+1 after comment. |
TL;DR
Currently setting overridable matchable resources requires configuring the json for a post request which can be cumbersome - especially when auth is enabled.
For more background on matchable resource types, how they're used and how they're applied - see managing customizable resources
Type
Are all requirements met?
Complete description
Tracking Issue
flyteorg/flyte#335
Follow-up issue
NA