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

component/pipeline name clash: compiles fine, but causes run-time error #2993

Closed
amygdala opened this issue Feb 5, 2020 · 6 comments
Closed
Assignees
Labels
area/sdk/dsl/compiler kind/bug status/triaged Whether the issue has been explicitly triaged

Comments

@amygdala
Copy link
Contributor

amygdala commented Feb 5, 2020

(This is with Build commit: ca58b22)

Running this pipeline gives the following run-time error, due to a name clash:
image

This issue was not caught at compile-time. And, I'd argue that it should be allowed and handled in some way (renaming the pipeline name in the yaml?) It breaks modularity to have to know the name of a component when loading it from a URL.

Here is the component definition.

@Ark-kun
Copy link
Contributor

Ark-kun commented Feb 6, 2020

Can you please check the SDK version. (print(kfp.__version__))
The issue should have been fixed in SDK version 0.1.38. #1555

@rmgogogo rmgogogo added the status/triaged Whether the issue has been explicitly triaged label Feb 6, 2020
@amygdala
Copy link
Contributor Author

amygdala commented Feb 6, 2020

hmm... I did need to update kfp.

However (not to sidetrack), while I can compile this example pipeline using kfp==0.1.38, when I compile the same pipeline using kfp>0.1.38 (including the latest 0.2.2), I get this error:

Traceback (most recent call last):
  File "viz_test_pipeline_nameclash.py", line 42, in <module>
    compiler.Compiler().compile(viz_test, __file__ + '.tar.gz')
  File "/Users/amyu/devrel/TensorFlow-World-Adv-Introduction-TF-Serving/tf-world-tf-serving2/lib/python3.7/site-packages/kfp/compiler/compiler.py", line 879, in compile
    package_path=package_path)
  File "/Users/amyu/devrel/TensorFlow-World-Adv-Introduction-TF-Serving/tf-world-tf-serving2/lib/python3.7/site-packages/kfp/compiler/compiler.py", line 941, in _create_and_write_workflow
    pipeline_conf)
  File "/Users/amyu/devrel/TensorFlow-World-Adv-Introduction-TF-Serving/tf-world-tf-serving2/lib/python3.7/site-packages/kfp/compiler/compiler.py", line 758, in _create_workflow
    pipeline_meta = _extract_pipeline_metadata(pipeline_func)
  File "/Users/amyu/devrel/TensorFlow-World-Adv-Introduction-TF-Serving/tf-world-tf-serving2/lib/python3.7/site-packages/kfp/dsl/_metadata.py", line 85, in _extract_pipeline_metadata
    component_spec = _extract_component_interface(func)
  File "/Users/amyu/devrel/TensorFlow-World-Adv-Introduction-TF-Serving/tf-world-tf-serving2/lib/python3.7/site-packages/kfp/components/_python_op.py", line 267, in _extract_component_interface
    type_struct = annotation_to_type_struct(parameter_annotation)
  File "/Users/amyu/devrel/TensorFlow-World-Adv-Introduction-TF-Serving/tf-world-tf-serving2/lib/python3.7/site-packages/kfp/components/_python_op.py", line 231, in annotation_to_type_struct
    annotation = annotation.to_dict()
TypeError: to_dict() missing 1 required positional argument: 'self'

Has the API changed?

@amygdala
Copy link
Contributor Author

amygdala commented Feb 6, 2020

Update: It looks like the compilation error comes from this String type spec syntax, which worked through 1.38:

from kfp.dsl.types import String
...
def viz_test(  
  input1: String = 'qwerty'
  ):

If I change that line to input1 = 'qwerty' it's fine.

(I know this is 'forking' the topic, sorry-- if this is a real issue I'll file a sep. bug).

@Ark-kun
Copy link
Contributor

Ark-kun commented Feb 6, 2020

Update: It looks like the compilation error comes from this String type spec syntax, which worked through 1.38:

I'll look into this.
As a workaround, please use the native str type annotation.

@amygdala
Copy link
Contributor Author

amygdala commented Feb 6, 2020

Thanks. And: at least with 0.1.38, the original 'name clash' issue is fixed. I assume that remains the case with newer releases.
So I'll close this, but if you open another issue for the compilation problem, can you cc me on it?

@amygdala amygdala closed this as completed Feb 6, 2020
@Ark-kun
Copy link
Contributor

Ark-kun commented Feb 6, 2020

I've looked into the String issue. Looks like the current SDK version only supports input1: String() = 'qwerty' (an instance of String, not the type itself).
I'll try to fix that.

magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this issue Oct 22, 2023
* Loosen ray range

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

* Fix import of RayServeHandle

ref ray-project/ray#34714

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

* Allow pandas 2.0

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

* Remove exclusion markers

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

* Run poetry lock --no-update

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

---------

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sdk/dsl/compiler kind/bug status/triaged Whether the issue has been explicitly triaged
Projects
None yet
Development

No branches or pull requests

3 participants