-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
SDK/DSL: Enable the deletion of a resource via ResourceOp method #3213
Conversation
Your approach is generic, but I have some questions from users' side:
Overall, it looks great and if we confirm users want to have a separate cc @elikatsis |
Did I get your arguments correct? |
@elikatsis Thank you for explanation! |
/retest |
I really need this one. it would be great if I can delete pvc in |
@kim-sardine It's also the feature that we want. @elikatsis Seems like one of the CI test failed? If you're tied up, perhaps I could help with it. |
/retest |
@PatrickXYS I don't understand what the error is. I tried running retest command but the bot didn't listen to me 😢 |
@elikatsis It's weird. Seems like this PR is detached from Prow system. I'm not sure if there's any workaround we could move forward. But submitting another PR should work. Perhaps mention our discussion in the new PR would help others gather context. |
947352d
to
183fd7d
Compare
* Add the method delete() to ResourceOps * Extend ResourceOp & VolumeOp tests Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>
183fd7d
to
cc26aed
Compare
/assign @Ark-kun |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun 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 |
…eflow#3213) * SDK/DSL: Enable the deletion of a resource via ResourceOp method * Add the method delete() to ResourceOps * Extend ResourceOp & VolumeOp tests Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com> * Fix ValueError not being raised
Related to #1779.
This PR enables performing the
delete
action on a resource that's highly connected to the workflow (e.g., a PVC created using aVolumeOp
).The approach of #3194 is aligned to this one, but is a subset of it. I decided to submit this PR to have a generic
delete()
method that would work out of the box with anyResourceOp
(or subclass).cc @PatrickXYS
This change is