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.

Sign up for GitHub

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

Jump to bottom

Add wait flag for Slurm mode #40

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Add wait flag for Slurm mode #40

merged 1 commit into from
Jan 22, 2025

Conversation

IrvingMg
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds an option to wait for the Slurm job to finish its execution.

Which issue(s) this PR fixes:

Fixes #21

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Adds an option to wait for Slurm jobs to finish their execution.

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Jan 15, 2025
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 15, 2025
mwysokin
mwysokin suggested changes Jan 17, 2025
View reviewed changes
Copy link

@mwysokin mwysokin left a comment

Choose a reason for hiding this comment

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

Seems like this PR needs a bit more polishing. There are some bugs and the last function doesn't handle pod failures. Please add those items.

@IrvingMg IrvingMg force-pushed the feature/add-wait-flag-for-slurm branch from 165ead2 to 9ff497e Compare January 19, 2025 19:33
@IrvingMg
Copy link
Contributor Author

/test all

mbobrovskyi
mbobrovskyi reviewed Jan 20, 2025
View reviewed changes
mbobrovskyi
mbobrovskyi reviewed Jan 20, 2025
View reviewed changes
@IrvingMg IrvingMg mentioned this pull request Jan 20, 2025
Open
3 tasks
@IrvingMg IrvingMg force-pushed the feature/add-wait-flag-for-slurm branch 2 times, most recently from 405aa5d to 1b10e64 Compare January 20, 2025 22:00
@IrvingMg
Copy link
Contributor Author

/test all

@IrvingMg IrvingMg marked this pull request as ready for review January 21, 2025 09:12
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 21, 2025
mbobrovskyi
mbobrovskyi reviewed Jan 21, 2025
View reviewed changes
mbobrovskyi
mbobrovskyi reviewed Jan 21, 2025
View reviewed changes
Copy link
Contributor

@mbobrovskyi mbobrovskyi left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Only few nits.

@IrvingMg IrvingMg force-pushed the feature/add-wait-flag-for-slurm branch from 81f8d4f to 00df293 Compare January 21, 2025 17:38
@IrvingMg IrvingMg force-pushed the feature/add-wait-flag-for-slurm branch from 00df293 to ba47f6d Compare January 22, 2025 09:02
@mbobrovskyi
Copy link
Contributor

/lgtm
Thank you!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 22, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 057ee31cf37e8e4c3e5864b7da5ea21923b7a675

@mbobrovskyi
Copy link
Contributor

/cc @mimowo

@k8s-ci-robot k8s-ci-robot requested a review from mimowo January 22, 2025 09:03
@mimowo
Copy link
Contributor

mimowo commented Jan 22, 2025

/approve
Thanks!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: IrvingMg, 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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2025
@k8s-ci-robot k8s-ci-robot merged commit 4f2cb10 into kubernetes-sigs:main Jan 22, 2025
6 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.2 milestone Jan 22, 2025
@IrvingMg IrvingMg deleted the feature/add-wait-flag-for-slurm branch January 22, 2025 10:03
@IrvingMg IrvingMg mentioned this pull request Jan 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbobrovskyi mbobrovskyi mbobrovskyi left review comments

@mwysokin mwysokin mwysokin requested changes

@mwielgus mwielgus Awaiting requested review from mwielgus

@mimowo mimowo Awaiting requested review from mimowo

Assignees

@mbobrovskyi mbobrovskyi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v0.2
Development

Successfully merging this pull request may close these issues.

Add --wait flag for Slurm mode.
5 participants
@IrvingMg @k8s-ci-robot @mbobrovskyi @mimowo @mwysokin