feat(sdk): ImporterSpec v2 #6917
Conversation
|
Skipping CI for Draft Pull Request. |
pipeline.yaml
Outdated
| @@ -0,0 +1,124 @@ | |||
| apiVersion: argoproj.io/v1alpha1 | |||
| @@ -1,164 +0,0 @@ | |||
| # Copyright 2020 The Kubeflow Authors | |||
There was a problem hiding this comment.
nit: ideally we should still keep a unit test file for this. Maybe test calling dsl.importer() with some value and then assert we got the right pipeline task object?
Given that we have the some test coverage in the compiler cli test, we could delay this a bit in a later PR.
There was a problem hiding this comment.
Okay, will add some tests in following PRs.
| deployment_config.executors[ | ||
| executor_label].container.CopyFrom( | ||
| subgroup_container_spec) | ||
| if hasattr(subgroup, 'container_spec'): |
There was a problem hiding this comment.
Given that we document both container_spec and importer_spec as class attributes (in the docstring of PipelineTask), hasattr seems to be a bit contradictory.
How about we initialize them to None. and check if subgroup.container_spec is not None here?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chensun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* importer * compiler * check diff * fix format * remove v1 node test * remove importer v2 compat test * release note * address comments
Add Importer Spec for v2.
The generated IR has the following difference with v1:
pipelineparam--dataset2topipelinechannel--dataset2typetoparameterType, addingdefaultValueetc.system.Artifact(will investigate further)