search

kubernetes-sigs / kueue

Kubernetes-native Job Queueing
https://kueue.sigs.k8s.io
Apache License 2.0
1.47k stars 262 forks source link

Use ResourceFlavorReference for ResourceFlavor names #3611

Closed kaisoz closed 12 hours ago

kaisoz commented 3 days ago

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

A ResourceFlavor name is referenced in different ways through the code, either as a ResourceFlavorReference or as a string. This PR ensures that only ResourceFlavorReference is used unless a cast to string is absolutely necessary.

Which issue(s) this PR fixes:

Fixes #3491

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE
netlify[bot] commented 3 days ago

Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
Latest commit d5a7bb776c0b99625553d4afc1892f86f3ffb202
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/674384266f05db00087ccac3
mbobrovskyi commented 3 days ago

/lgtm Awesome. Thanks! As a follow-up, it would be nice to update the tests to use the .Ref() method.

k8s-ci-robot commented 3 days ago

LGTM label has been added.

Git tree hash: fcf16510009eea9a1935d2da5e540d6c79faf1c0

kaisoz commented 3 days ago

/lgtm Awesome. Thanks! As a follow-up, it would be nice to update the tests to use the .Ref() method.

Thanks! 😊 I see that other reviewers have concerns on the method... what's your take on it? It feels that I should go with the cast to ResourceFlavorReference and then assess?

mbobrovskyi commented 3 days ago

/lgtm Awesome. Thanks! As a follow-up, it would be nice to update the tests to use the .Ref() method.

Thanks! 😊 I see that other reviewers have concerns on the method... what's your take on it? It feels that I should go with the cast to ResourceFlavorReference and then assess?

Yes, please, cast to ResourceFlavorReference 🙏

kaisoz commented 3 days ago

@mbobrovskyi @mimowo @tenzen-y changes added. PTAL 😊 Thanks for you time!

k8s-ci-robot commented 13 hours ago

LGTM label has been added.

Git tree hash: 980904c03b4118e6f9b24b8543eacd2a7b33eade

mimowo commented 13 hours ago

/approve Thanks!

k8s-ci-robot commented 13 hours ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaisoz, mbobrovskyi, mimowo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kueue/blob/main/OWNERS)~~ [mimowo] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment