Skip to content

Feat/make auto emit events configurable from argo UI #2782

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

Open
tfurmston wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Feat/make auto emit events configurable from argo UI #2782

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
313ddd5
Add the ability to toggle auto_emit_argo_events from the argo Workflo…
tfurmston Jan 27, 2026
95cc494
Move construction of argo event parameter
tfurmston Jan 28, 2026
7b4ee51
remove stray import
tfurmston Jan 28, 2026
cca651e
remove stray import
tfurmston Jan 28, 2026
d3c00b9
fix formatting and use int instead of boolean
tfurmston Jan 28, 2026
5ea7c67
run precommit
tfurmston Jan 29, 2026
ffd2a8e
Merge branch 'master' into feat/make_auto_emit_events_configurable_fr…
tfurmston Mar 19, 2026
86ad07c
Merge branch 'master' into feat/make_auto_emit_events_configurable_fr…
tfurmston Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions metaflow/parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ def init(self, ignore_errors=False):
"run-id",
"task-id",
"runner-attribute-file",
"auto-emit-argo-events",
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 15, 2026

Choose a reason for hiding this comment

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

Argo-specific reserved name leaks into general parameters.py

Adding "auto-emit-argo-events" (and implicitly "auto_emit_argo_events" via the hyphen-to-underscore conversion on line 417) to the global reserved parameter list in parameters.py couples a general-purpose Metaflow module to an Argo Workflows-specific concept.

This was one of the two explicit reasons cited for reverting PR #2770: "adding argo-specific references to parameters.py is undesirable".

Consider keeping this reservation within the Argo plugin itself — for example, by overriding a _reserved_parameter_names hook in ArgoWorkflows (or its CLI) rather than hard-coding Argo-specific names in the shared parameters.py.

]
reserved = set(reserved_params)
# due to the way Click maps cli args to function args we also want to add underscored params to the set
Expand Down
13 changes: 9 additions & 4 deletions metaflow/plugins/argo/argo_workflows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,13 @@ def _compile_workflow_template(self):
.description("auto-set by metaflow. safe to ignore.")
for event in self.triggers
]
+ [
Parameter("auto_emit_argo_events")
.value(1 if self.auto_emit_argo_events else 0)
.description(
"Toggle used to control emission of events to Argo Events."
)
Comment on lines +971 to +973
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 19, 2026

Choose a reason for hiding this comment

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

P2 Parameter description does not document valid values

The description "Toggle used to control emission of events to Argo Events." doesn't tell users which values are accepted. The decorator parser in decorators.py (extract_args_kwargs_from_decorator_spec) first attempts json.loads, then int, then float, falling back to a raw string. Most common inputs ("0", "1", "true", "false") parse correctly to falsy/truthy values, but a user who enters "no", "off", or "disabled" will end up with a non-empty string that evaluates as truthy — silently keeping event emission enabled despite their intent.

Updating the description to explicitly state the accepted values would prevent this confusion:

Suggested change
.description(
"Toggle used to control emission of events to Argo Events."
)
.description(
"Toggle used to control emission of events to Argo Events. Set to 1 to enable, 0 to disable."
)

]
Comment on lines +968 to +974
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 15, 2026

Choose a reason for hiding this comment

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

Parameter always added, regardless of Argo Events usage

The auto_emit_argo_events parameter is appended unconditionally to every Argo workflow's .arguments(), so it will appear in the Argo UI for all deployed flows — even those that have no Argo Events integration and where auto_emit_argo_events was False at deployment time.

This is the same root issue that caused the previous incarnation of this change (PR #2770) to be reverted: it "introduc[es] a new always-present parameter to flows on Argo that will be confusing to users".

If the intent is to expose the toggle only for flows that actually use Argo Events (or that were deployed with auto_emit_argo_events=True), the block should be guarded:

# Only expose the toggle when the workflow was deployed with Argo Events awareness
+ ([
+     Parameter("auto_emit_argo_events")
+     .value(1 if self.auto_emit_argo_events else 0)
+     .description(
+         "Toggle used to control emission of events to Argo Events."
+     )
+ ] if self.triggers or self.auto_emit_argo_events else [])

Without this guard, every Metaflow user running on Argo will see an unfamiliar auto_emit_argo_events parameter that is irrelevant to their workflow, which re-introduces the exact problem that led to the prior revert.

)
)
# Set common pod metadata.
Expand Down Expand Up @@ -2151,8 +2158,7 @@ def _container_templates(self):
"--event-logger=%s" % self.event_logger.TYPE,
"--monitor=%s" % self.monitor.TYPE,
"--no-pylint",
"--with=argo_workflows_internal:auto-emit-argo-events=%i"
% self.auto_emit_argo_events,
"--with=argo_workflows_internal:auto-emit-argo-events={{workflow.parameters.auto_emit_argo_events}}",
]

if node.name == "start":
Expand Down Expand Up @@ -3605,8 +3611,7 @@ def _heartbeat_daemon_template(self):
"--event-logger=%s" % self.event_logger.TYPE,
"--monitor=%s" % self.monitor.TYPE,
"--no-pylint",
"--with=argo_workflows_internal:auto-emit-argo-events=%i"
% self.auto_emit_argo_events,
"--with=argo_workflows_internal:auto-emit-argo-events={{workflow.parameters.auto_emit_argo_events}}",
]
heartbeat_cmds = "{entrypoint} {top_level} argo-workflows heartbeat --run_id {run_id} {tags}".format(
entrypoint=" ".join(entrypoint),
Expand Down
Loading