Second stab at deploy time trigger on finish bugs #2212
Conversation
There was a problem hiding this comment.
not sure which change causes this, but there is also an issue with the following example on argo workflows:
def flow_names_func(ctx):
return ["DeployTimeTriggerParams"]
@trigger_on_finish(flows=flow_names_func)
class DeployTimeTriggerOnFinishFlow2(FlowSpec):as for whatever reason the trigger now becomes just a string, leading to:
argo_workflows.py", line 635, in _process_triggers
event.get("project") or current.get("project_name"),
AttributeError: 'str' object has no attribute 'get'
| if "project" not in trigger: | ||
| raise MetaflowException( | ||
| f"Trigger: {trigger} parsed at deploy time is missing the *project* key." | ||
| ) | ||
| if "project_branch" not in trigger: | ||
| raise MetaflowException( | ||
| f"Trigger: {trigger} parsed at deploy time is missing the *project_branch* key." | ||
| ) |
There was a problem hiding this comment.
this seems like a clear change in behaviour. previously partial dictionaries were accepted, though supplying a project without a branch lead to nothing being triggered. a dict with only a name used to be valid though
There was a problem hiding this comment.
Yes -- @talsperre , why are we forcing this now? It should be auto-filled with current project/branch if not provided right?
|
Can we add some unit tests for this logic? It's really complicated and hard to verify correctness just by eyeballing |
No description provided.