-
Notifications
You must be signed in to change notification settings - Fork 38
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
fix(delete_workflow): set retention rules as inactive (#595) #595
fix(delete_workflow): set retention rules as inactive (#595) #595
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #595 +/- ##
==========================================
+ Coverage 75.11% 75.13% +0.01%
==========================================
Files 15 15
Lines 1475 1476 +1
==========================================
+ Hits 1108 1109 +1
Misses 367 367
|
88e40fe
to
8422040
Compare
8422040
to
bc614b8
Compare
int_session = workflow.sessions.first() | ||
if int_session: | ||
kwrm = KubernetesWorkflowRunManager(workflow) | ||
kwrm.stop_interactive_session(int_session.id_) | ||
|
||
if workspace: | ||
# 2. disable retention rules, as the workspace will be deleted | ||
workflow.inactivate_workspace_retention_rules() | ||
# 3. delete the workspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment: wouldn't it be better to change the order of 2 and 3 so that the actual deletion of files will occur first? Thinking about a possible corner case what if something goes wrong with the retention rule inactivation: at least the files would be deleted by the time we get an exception from that function if we switch the order. (Assuming that the most frequent use case is for the researcher to liberate some disk space.)
bc614b8
to
680e288
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well 👍
$ grep -C 1 retention reana.yaml
workspace:
retention_days:
'*.root': 10
$ reana-client run -w test
$ sleep 60
$ reana-client status -w test | grep -cw finished
1
$ reana-client delete -w test
$ kubectl create job --from cronjob/reana-retention-rules-apply reana-retention-rules-apply2
job.batch/reana-retention-rules-apply2 created
$ kubectl logs reana-retention-rules-apply2-p5d9n
Setting the status of all the rules that will be applied to `pending`
Fetching all the pending rules
No rules to be applied!
Closes reanahub/reana-server#693