Feat: Add JobObjectMeta to plugins flyteidl #6319
Conversation
Signed-off-by: Fabio Grätz <[email protected]>
Code Review Agent Run Status
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6319 +/- ##
==========================================
- Coverage 58.50% 58.49% -0.01%
==========================================
Files 937 937
Lines 71088 71088
==========================================
- Hits 41587 41581 -6
- Misses 26349 26355 +6
Partials 3152 3152
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -25,3 +26,10 @@ message CommonReplicaSpec { | |||
| // RestartPolicy determines whether pods will be restarted when they exit | |||
| RestartPolicy restart_policy = 4; | |||
| } | |||
|
|
|||
| // Object metadata applied to the CRD object underlying a task execution | |||
| message JobObjectMeta { | |||
There was a problem hiding this comment.
Why is this a new object? We have labels and annotations right?
Why not part of task template
There was a problem hiding this comment.
You are right, I should reuse message K8sObjectMetadata { 🤔
There was a problem hiding this comment.
Why not part of task template
For normal python function tasks you can set custom pod labels/annotations via the pod template arg of the task decorator.
As described in #6238 I'm looking to add a way to allow setting labels/annotations to CRD objects of plugins like kubeflow, dask, spark, ray, ... (because I want to use Kueue which requires an annotation on the job):
@task(
task_config=PyTorch(
num_workers=...,
...
# Proposed addition:
metadata=ObjectMeta(
annotations={"kueue.x-k8s.io/queue-name": "queue-name"},
labels={...}
)
)
)
Pod template doesn't work because this sets it on the worker pods, not the CRD object.
Does it sound reasonable to you to add the existing K8sObjectMetadata to the distributed job proto messages like DistributedPyTorchTrainingTask (see here)?
Tracking issue
Related to #6238
Why are the changes needed?
What changes were proposed in this pull request?
How was this patch tested?
Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link