feat(sdk): update component deserialization logic for custom artifacts (support custom artifact types pt. 3) #8181
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
connor-mccarthy
force-pushed
the
support-deserializing-component-inputs
branch
from
02951fb to
220269b
Compare
connor-mccarthy
force-pushed
the
support-deserializing-component-inputs
branch
from
220269b to
65e789b
Compare
| type_ = 'Artifact' | ||
| outputs[utils.sanitize_input_name( | ||
| spec['name'])] = OutputSpec(type=type_) | ||
|
|
There was a problem hiding this comment.
These changes bring the v1 deserialization logic into alignment with the serialization logic (which falls back to system.Artifact, that way the read/write tests pass.
| name: InputSpec.from_ir_parameter_dict(parameter_dict) | ||
| for name, parameter_dict in parameters.items() | ||
| } | ||
|
|
||
| artifacts = components_dict[component_key].get( |
There was a problem hiding this comment.
These are the changes required to correctly handle deserialization of components with artifact inputs.
This is the main reason for this PR. The other changes are to make the serialization/deserialization logic consistent w.r.t. treatment of artifacts. This inconsistency made it very hard for me to test changes related to #8172.
| pipeline_func=my_pipeline, package_path='result.yaml') | ||
| pipeline_func=my_pipeline, package_path=target_yaml_file) | ||
|
|
||
| self.assertTrue(os.path.exists(target_yaml_file)) |
There was a problem hiding this comment.
These changes were needed to align with the information compression that happens for artifacts (unknown artifacts get compressed to schema_title='system.Artifact').
This exception will not be thrown when SomeArbitraryArtifact gets loaded as system.Artifact.
|
/retest |
1 similar comment
|
/retest |
connor-mccarthy
changed the title
|
/retest |
|
@connor-mccarthy: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
connor-mccarthy
changed the title
Description of your changes:
This PR fixes component deserialization logic for components with input and output
artifacts.
Currently, the SDK cannot deserialize components with artifact inputs and does
not handle inputs and outputs correctly when deserializing v1 component YAML. This
fix is a requirement for supporting custom artifact types.
Checklist:
Learn more about the pull request title convention used in this repository.