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

feat(experiment): Add a check before deletion #1223

Merged
merged 3 commits into from
Jul 31, 2020
Merged

feat(experiment): Add a check before deletion #1223

merged 3 commits into from
Jul 31, 2020

Conversation

gaocegege
Copy link
Member

Signed-off-by: Ce Gao gaoce@caicloud.io

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1222

Special notes for your reviewer:

  1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

@kubeflow-bot
Copy link

This change is Reviewable

gaocegege
gaocegege commented Jun 19, 2020
View reviewed changes
Copy link
Member Author

@gaocegege gaocegege left a comment

Choose a reason for hiding this comment

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

/assign @andreyvelich @johnugeorge

andreyvelich
andreyvelich reviewed Jun 19, 2020
View reviewed changes
pkg/controller.v1alpha3/experiment/experiment_controller.go Outdated
@@ -18,6 +18,7 @@ package experiment

Copy link
Member

@andreyvelich andreyvelich Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Do we want to make this change in v1alpha3 version since we already created a release?
What do you think @gaocegege @johnugeorge ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Depends on if we maintain v1alpha3 in the next release. I think we should keep for 1 more release since the v1beta1 may be not stable. WDYT @johnugeorge

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @idahoakl

PTAL, thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

@idahoakl PTAL, thanks.

@gaocegege gaocegege mentioned this pull request Jul 28, 2020
Closed
@gaocegege
Copy link
Member Author

/retest

@andreyvelich
Copy link
Member

Thanks @gaocegege, can you also add this change for v1beta1 and we can merge it.
/cc @idahoakl @johnugeorge

@k8s-ci-robot k8s-ci-robot requested a review from johnugeorge July 30, 2020 21:54
@k8s-ci-robot
Copy link

@andreyvelich: GitHub didn't allow me to request PR reviews from the following users: idahoakl.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Thanks @gaocegege, can you also add this change for v1beta1 and we can merge it.
/cc @idahoakl @johnugeorge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gaocegege
Copy link
Member Author

OK, will do it.

gaocegege added 3 commits July 31, 2020 12:32
@gaocegege
feat(experiment): Add a check before deletion
ff31a95 
Signed-off-by: Ce Gao <gaoce@caicloud.io>
@gaocegege
fix: Delete all trials
e021673 
Signed-off-by: Ce Gao <gaoce@caicloud.io>
@gaocegege
feat: Implement in v1beta1
ab52e34 
Signed-off-by: Ce Gao <gaoce@caicloud.io>
@gaocegege
Copy link
Member Author

PTAL @andreyvelich

@johnugeorge
Copy link
Member

/lgtm

andreyvelich
andreyvelich approved these changes Jul 31, 2020
View reviewed changes
Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thanks @gaocegege
/approve
/lgtm

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich

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 merged commit ce89cbf into kubeflow:master Jul 31, 2020
@andreyvelich andreyvelich mentioned this pull request Jul 31, 2020
Merged
1 task
@gaocegege gaocegege deleted the v1alpha3restart branch September 9, 2020 08:09
whalecold pushed a commit to whalecold/katib that referenced this pull request Sep 16, 2020
@gaocegege @whalecold
feat(experiment): Add a check before deletion (kubeflow#1223)
6ada9a7 
* feat(experiment): Add a check before deletion

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* fix: Delete all trials

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* feat: Implement in v1beta1

Signed-off-by: Ce Gao <gaoce@caicloud.io>
whalecold pushed a commit to whalecold/katib that referenced this pull request Sep 16, 2020
feat(experiment): Add a check before deletion (kubeflow#1223)
d0f3544 
* feat(experiment): Add a check before deletion

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* fix: Delete all trials

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* feat: Implement in v1beta1

Signed-off-by: Ce Gao <gaoce@caicloud.io>
@whalecold whalecold mentioned this pull request Sep 16, 2020
Merged
caicloud-bot pushed a commit to caicloud/katib that referenced this pull request Sep 16, 2020
@whalecold
feat(experiment): Add a check before deletion (kubeflow#1223) (#11)
1ea2cd5 
* feat(experiment): Add a check before deletion

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* fix: Delete all trials

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* feat: Implement in v1beta1

Signed-off-by: Ce Gao <gaoce@caicloud.io>

Co-authored-by: whalecold <zhenglisheng@caicloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreyvelich andreyvelich andreyvelich approved these changes

@Akado2009 Akado2009 Awaiting requested review from Akado2009

@jinan-zhou jinan-zhou Awaiting requested review from jinan-zhou

@johnugeorge johnugeorge Awaiting requested review from johnugeorge

Assignees

@johnugeorge johnugeorge

@andreyvelich andreyvelich

Labels
approved lgtm size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bug] Runtime panic in v1alpha3 when deleting extra trials
5 participants
@gaocegege @kubeflow-bot @andreyvelich @k8s-ci-robot @johnugeorge