Skip to content
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

add function for listing lqs by flavors #710

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

KPostOffice
Copy link
Collaborator

@KPostOffice KPostOffice commented Oct 15, 2024

Issue link

https://issues.redhat.com/browse/RHOAIENG-13429

What changes have been made

This lists local queues available to a user as well as the flavors for that LQ if available

Verification steps

make and deploy kueue with latest changes from opendatahub-io/kueue#52

Create Flavors, LQs, and CQ

from codeflare_sdk.common.kueue import list_local_queues

list_local_queues(...)

will update with more details when cluster resumes

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

Sorry, something went wrong.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 15, 2024
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.06%. Comparing base (7c04444) to head (e1cedb1).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #710      +/-   ##
==========================================
+ Coverage   93.99%   94.06%   +0.07%     
==========================================
  Files          36       36              
  Lines        2364     2394      +30     
==========================================
+ Hits         2222     2252      +30     
  Misses        142      142              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KPostOffice KPostOffice force-pushed the list-lqs branch 4 times, most recently from 7bbdc70 to 905d556 Compare October 16, 2024 15:56
@KPostOffice KPostOffice changed the title WIP: add function for listing lqs by flavors add function for listing lqs by flavors Oct 16, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 16, 2024
Copy link
Contributor

@Bobbins228 Bobbins228 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KPostOffice Can you provide some verification steps? I am going to take a stab at testing this and I want to make sure I am setting up my environment correctly.
Thanks

Copy link
Contributor

@Ygnas Ygnas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as intended.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2024
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2024
Copy link
Contributor

@Bobbins228 Bobbins228 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a simple import for this function?

Signed-off-by: Kevin <kpostlet@redhat.com>
Copy link
Contributor

@Bobbins228 Bobbins228 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KPostOffice Works perfectly couple of notes.

  1. Failed on my cluster with regular user possibly because I am using an independent Kueue deployment. (verified a regular user can see localqueues on another clean cluster)
    /lgmt /approve

Great work!

Copy link
Contributor

@Bobbins228 Bobbins228 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm /approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2024
Copy link
Contributor

openshift-ci bot commented Oct 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobbins228, Ygnas

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 17, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit b7c37af into project-codeflare:main Oct 17, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants