Skip to content

Conversation

cloudw
Copy link

@cloudw cloudw commented Aug 24, 2024

Parameters with no value is not valid for Workflows but are valid for WorkflowTemplate. Without being added to WorkflowTemplate, these parameter will not be accepted from Argo UI or sensors.

@cloudw cloudw requested a review from talebzeghmi August 24, 2024 04:35
@cloudw cloudw self-assigned this Aug 24, 2024
@cloudw cloudw force-pushed the yunw/AIP-8651-include-all-params branch from 4ef1ef0 to 73521f2 Compare August 24, 2024 04:36

import boto3
import pytest
from botocore.exceptions import ClientError
Copy link
Author

Choose a reason for hiding this comment

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

Unused imports.

for param in self.flow._get_parameters()
if param not in flow_parameters
]
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this may not work:

File "/home/zservice/metaflow/metaflow/task.py", line 548, in run_step
    self._exec_step_function(step_func)
  File "/home/zservice/metaflow/metaflow/task.py", line 53, in _exec_step_function
    step_function()
  File "/home/zservice/metaflow/metaflow/plugins/aip/tests/flows/flow_triggering_flow.py", line 89, in start
    self.submit_template(path)
  File "/home/zservice/metaflow/metaflow/plugins/aip/tests/flows/flow_triggering_flow.py", line 198, in submit_template
    subprocess.run(
  File "/usr/local/lib/python3.9/subprocess.py", line 528, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['argo', 'template', '-n', 'metaflow-integration-testing-internal', 'create', '/tmp/wfdsk-ftf-test-argo-d64fe9ff-f1d7-4c7c-8551-c45c3c600063-0.yaml']' returned non-zero exit status 1.

see:

Copy link
Collaborator

Choose a reason for hiding this comment

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

note: it'll also be important to ensure it's tested with a workflow compiled with an exit handler to ensure we don't hit argoproj/argo-workflows#6036

@cloudw cloudw force-pushed the yunw/AIP-8651-include-all-params branch from afef70b to ab6a17f Compare September 22, 2024 23:43
@cloudw cloudw force-pushed the yunw/AIP-8651-include-all-params branch from ab6a17f to 0738816 Compare October 2, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants