Skip to content
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

feat(backend): enable dsl.Collected for parameters & artifacts #11725

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zazulam
Copy link
Contributor

@zazulam zazulam commented Mar 5, 2025

Description of your changes:
Jumping off from #11627, these changes resolve #10050 in regards to the using dsl.Collected for both parameters and artifacts in pipelines. Currently for artifacts, the executor needs to be updated in the sdk and have a release prior to tests being enabled. As for parameters that should work out of the box with solely backend changes.

Checklist:

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign chensun for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zazulam
Copy link
Contributor Author

zazulam commented Mar 5, 2025

Providing some more context, this is the error that occurs for artifacts when a user attempts use either List[Artifact]:

main     executor = component_executor.Executor(
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 52, in __init__
main     self.assign_input_and_output_artifacts()
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 73, in assign_input_and_output_artifacts
main     self.input_artifacts[name] = [
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 74, in <listcomp>
main     self.make_artifact(
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 118, in make_artifact
main     return create_artifact_instance(
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 378, in create_artifact_instance
main     ) if hasattr(artifact_cls, '_from_executor_fields') else artifact_cls(
main   File "/usr/local/lib/python3.9/typing.py", line 685, in __call__
main     raise TypeError(f"Type {self._name} cannot be instantiated; "
main TypeError: Type List cannot be instantiated; use list() instead

or list[Artifact]

main     return _run_code(code, main_globals, None,
main   File "/usr/local/lib/python3.9/runpy.py", line 87, in _run_code
main     exec(code, run_globals)
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor_main.py", line 109, in <module>
main     executor_main()
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor_main.py", line 98, in executor_main
main     executor = component_executor.Executor(
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 52, in __init__
main     self.assign_input_and_output_artifacts()
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 73, in assign_input_and_output_artifacts
main     self.input_artifacts[name] = [
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 74, in <listcomp>
main     self.make_artifact(
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 118, in make_artifact
main     return create_artifact_instance(
main   File "/usr/local/lib/python3.9/site-packages/kfp/dsl/executor.py", line 378, in create_artifact_instance
main     ) if hasattr(artifact_cls, '_from_executor_fields') else artifact_cls(
main TypeError: list() takes no keyword arguments

So we use the get_args & get_origin to determine the actual artifact cls type used within the collection.

I had made a custom image to test with the sdk installed with the executor changes, this is what it looks like:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[sdk] Unable to aggregate results over ParallelFor in Kubeflow V2 using V1 workarounds such as .after()
1 participant