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

Orchestration/Kubeflow - Removed the use of undocumented KFP compiler API #39

Merged
merged 2 commits into from
Apr 23, 2019

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Apr 11, 2019

See #30 (comment)

The KFP documentation clearly says that the pipeline function must be decorated with @pipeline decorator. It's even mentioned in the compile function documentation: https://github.com/kubeflow/pipelines/blob/086d4763d9f163acdeb423bc2bc49ab470442a92/sdk/python/kfp/compiler/compiler.py#L636

Running Compiler().compile(...) without having the function decorated will raise error: https://github.com/kubeflow/pipelines/blob/086d4763d9f163acdeb423bc2bc49ab470442a92/sdk/python/kfp/compiler/compiler.py#L578 'Please use a function with @dsl.pipeline decorator.'.

For some reason this documented behavior was circumvented by a hack that used an undocumented function that should be a part of internal compiler API.

This has triggered the #30 issue when the internal compiler API was changed by the KFP team.

This PR fixes this issue.

See tensorflow#30 (comment)

The KFP documentation clearly says that the pipeline function must be decorated with `@pipeline` decorator. It's even mentioned in the `compile` function documentation: https://github.com/kubeflow/pipelines/blob/086d4763d9f163acdeb423bc2bc49ab470442a92/sdk/python/kfp/compiler/compiler.py#L636

Running `compile` without having the function decorated
https://github.com/tensorflow/tfx/blob/d9f87b9b03ab8bb98653b8a96d5bf952bc22b2b7/tfx/orchestration/kubeflow/runner.py#L125 will raise error: https://github.com/kubeflow/pipelines/blob/086d4763d9f163acdeb423bc2bc49ab470442a92/sdk/python/kfp/compiler/compiler.py#L578 `'Please use a function with @dsl.pipeline decorator.'`.

For some reason this documented behavior was circumvented by a hack that used an undocumented function that should be a part of internal compiler API.

This PR fixes this issue.
@Ark-kun Ark-kun mentioned this pull request Apr 11, 2019
@Ark-kun Ark-kun changed the title Removed the use of undocumented compiler API Orchestration/Kubeflow - Removed the use of undocumented KFP compiler API Apr 11, 2019
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 13, 2019

/cc @neuromage

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 17, 2019

Gentle ping

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 18, 2019

Ping.

@zhitaoli zhitaoli self-assigned this Apr 19, 2019
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 22, 2019

JFYI: The decorator is not required by the KFP compiler since v0.1.17 (last week release).

Also:

@decorator(args)
def func(...)
    ...

is syntactic sugar for

def func(...):
    ...

func = decorator(args)(func)

@tfx-copybara tfx-copybara merged commit 42ab481 into tensorflow:master Apr 23, 2019
tfx-copybara pushed a commit that referenced this pull request Apr 23, 2019
PiperOrigin-RevId: 244882204
@Ark-kun Ark-kun deleted the patch-1 branch April 23, 2019 21:33
ruoyu90 pushed a commit to ruoyu90/tfx that referenced this pull request Aug 28, 2019
This fix update the SIG IO's release process so that
anyone could help the release for SIG IO.

Signed-off-by: Yong Tang <[email protected]>
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.

5 participants