-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: add TTL strategy to Argo WorkflowTemplates #3104
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
base: master
Are you sure you want to change the base?
Changes from all commits
90f55bc
b412523
510a5ba
21bb155
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |||||||||||||||||||||||||||||||||||||||
| ARGO_WORKFLOWS_CAPTURE_ERROR_SCRIPT, | ||||||||||||||||||||||||||||||||||||||||
| ARGO_WORKFLOWS_ENV_VARS_TO_SKIP, | ||||||||||||||||||||||||||||||||||||||||
| ARGO_WORKFLOWS_KUBERNETES_SECRETS, | ||||||||||||||||||||||||||||||||||||||||
| ARGO_WORKFLOWS_TTL_SECONDS_AFTER_COMPLETION, | ||||||||||||||||||||||||||||||||||||||||
| ARGO_WORKFLOWS_UI_URL, | ||||||||||||||||||||||||||||||||||||||||
| AWS_SECRETS_MANAGER_DEFAULT_REGION, | ||||||||||||||||||||||||||||||||||||||||
| AZURE_KEY_VAULT_PREFIX, | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -897,6 +898,8 @@ def _compile_workflow_template(self): | |||||||||||||||||||||||||||||||||||||||
| WorkflowSpec() | ||||||||||||||||||||||||||||||||||||||||
| # Set overall workflow timeout. | ||||||||||||||||||||||||||||||||||||||||
| .active_deadline_seconds(self.workflow_timeout) | ||||||||||||||||||||||||||||||||||||||||
| # Set TTL for completed workflows (default: 7 days). | ||||||||||||||||||||||||||||||||||||||||
| .ttl_strategy(ARGO_WORKFLOWS_TTL_SECONDS_AFTER_COMPLETION) | ||||||||||||||||||||||||||||||||||||||||
| # TODO: Allow Argo to optionally archive all workflow execution logs | ||||||||||||||||||||||||||||||||||||||||
| # It's disabled for now since it requires all Argo installations | ||||||||||||||||||||||||||||||||||||||||
| # to enable an artifactory repository. If log archival is | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -4295,6 +4298,14 @@ def hooks(self, hooks): | |||||||||||||||||||||||||||||||||||||||
| self.payload["hooks"].update({k: v.to_json()}) | ||||||||||||||||||||||||||||||||||||||||
| return self | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def ttl_strategy(self, seconds_after_completion): | ||||||||||||||||||||||||||||||||||||||||
| # https://argoproj.github.io/argo-workflows/fields/#ttlstrategy | ||||||||||||||||||||||||||||||||||||||||
| if seconds_after_completion is not None and int(seconds_after_completion) > 0: | ||||||||||||||||||||||||||||||||||||||||
| self.payload["ttlStrategy"] = { | ||||||||||||||||||||||||||||||||||||||||
| "secondsAfterCompletion": int(seconds_after_completion) | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| return self | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4301
to
+4307
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def to_json(self): | ||||||||||||||||||||||||||||||||||||||||
| return self.payload | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| """Tests for WorkflowSpec.ttl_strategy().""" | ||
|
|
||
| from metaflow.plugins.argo.argo_workflows import WorkflowSpec | ||
|
|
||
|
|
||
| def test_positive_value_sets_ttl(): | ||
| ws = WorkflowSpec() | ||
| ws.ttl_strategy(604800) | ||
| assert ws.payload["ttlStrategy"] == {"secondsAfterCompletion": 604800} | ||
|
|
||
|
|
||
| def test_zero_disables_ttl(): | ||
| ws = WorkflowSpec() | ||
| ws.ttl_strategy(0) | ||
| assert "ttlStrategy" not in ws.payload | ||
|
|
||
|
|
||
| def test_none_disables_ttl(): | ||
| ws = WorkflowSpec() | ||
| ws.ttl_strategy(None) | ||
| assert "ttlStrategy" not in ws.payload | ||
|
|
||
|
|
||
| def test_negative_disables_ttl(): | ||
| ws = WorkflowSpec() | ||
| ws.ttl_strategy(-1) | ||
| assert "ttlStrategy" not in ws.payload | ||
|
|
||
|
|
||
| def test_string_value_converted_to_int(): | ||
| ws = WorkflowSpec() | ||
| ws.ttl_strategy("86400") | ||
| assert ws.payload["ttlStrategy"] == {"secondsAfterCompletion": 86400} | ||
|
|
||
|
|
||
| def test_chaining(): | ||
| """ttl_strategy returns self for method chaining.""" | ||
| ws = WorkflowSpec() | ||
| result = ws.ttl_strategy(3600) | ||
| assert result is ws |
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.
The default of 7 days means every existing Metaflow user who upgrades will silently start having their completed Argo Workflow resources auto-deleted after 7 days. Users who rely on workflow history beyond that window (e.g. for audit, debugging, or custom tooling that queries old workflow objects) will lose it without any explicit action. A safer default might be
Noneor0(disable TTL) to preserve the existing behavior, with opt-in for cleanup.