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

SDK - Unified the function signature parsing implementations #2689

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Dec 3, 2019

Problem: The SDK had were three different code paths that that parsed the python function signatures:

  • _extract_component_interface which was used by func_to_container_op
  • _extract_pipeline_metadata which was used by the DSL compiler during the compilation
  • _extract_component_metadata which was used by the @component decorator

The parsing code was pretty similar, but also had inconsistencies. This leads to problems (for example when the pipeline parameter types were parsed differently from the component input types).

This PR unifies the signature parsing code paths.
This is mostly a refactoring, so it does not introduce any changes for the SDK user (apart from fixing the inconsistency bugs).

Closes #2533


This change is Reviewable

sdk/python/kfp/components/_python_op.py Outdated Show resolved Hide resolved
sdk/python/kfp/components/_python_op.py Outdated Show resolved Hide resolved
sdk/python/kfp/dsl/_metadata.py Outdated Show resolved Hide resolved
sdk/python/kfp/dsl/types.py Outdated Show resolved Hide resolved
sdk/python/kfp/dsl/types.py Show resolved Hide resolved
@numerology
Copy link

/lgtm

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Dec 18, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ark-kun

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@numerology
Copy link

/retest

@k8s-ci-robot k8s-ci-robot merged commit 27f7e77 into kubeflow:master Dec 27, 2019
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this pull request Dec 9, 2020
…w#2689)

* Replaced `_instance_to_dict(obj)` with `obj.to_dict()`

* Fixed the capitalization in _python_function_name_to_component_name
It now only changes the case of the first letter.

* Replaced the _extract_component_metadata function with _extract_component_interface

* Stopped adding newline to the component description.

* Handling None inputs and outputs

* Not including emply inputs and outputs in component spec

* Renamed the private attributes that the @pipeline decorator sets

* Changged _extract_pipeline_metadata to use _extract_component_interface

* Fixed issues based on feedback
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023
…flow#2689)

* Bump github.com/emicklei/go-restful

Bumps [github.com/emicklei/go-restful](https://github.com/emicklei/go-restful) from 2.9.5+incompatible to 2.16.0+incompatible.
- [Release notes](https://github.com/emicklei/go-restful/releases)
- [Changelog](https://github.com/emicklei/go-restful/blob/v3/CHANGES.md)
- [Commits](emicklei/go-restful@v2.9.5...v2.16.0)

---
updated-dependencies:
- dependency-name: github.com/emicklei/go-restful
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update requirements.txt

Signed-off-by: Dan Sun <dsun20@bloomberg.net>

* Update xgb.Dockerfile

Signed-off-by: Dan Sun <dsun20@bloomberg.net>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Dan Sun <dsun20@bloomberg.net>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Dan Sun <dsun20@bloomberg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the same code to parse pipeline and component function signatures
3 participants