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

Update component.yaml to kfp v2 sdk #259

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

yhwang
Copy link
Contributor

@yhwang yhwang commented Aug 11, 2021

Update component.yaml to kfp v2 compatible. In v2,
you need to declare the data type for all of the input/output
arguments.

Signed-off-by: Yihong Wang yh.wang@ibm.com

Update component.yaml to kfp v2 compatible. In v2,
you need to declare the data type for all of the input/output
arguments.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>
@yhwang
Copy link
Contributor Author

yhwang commented Aug 11, 2021

/cc @Tomcli

@Tomcli
Copy link
Contributor

Tomcli commented Aug 11, 2021

So KFP is working on a new v2 design that requires few extra fields from its components. Part of the requirements is to specify the input/output parameter type for all the KFP components for the new design. This PR is to address this change.

@nrkarthikeyan
Copy link
Collaborator

@Tomcli , @yhwang - does this change preserve backward compatibility?

@Tomcli
Copy link
Contributor

Tomcli commented Sep 7, 2021

@Tomcli , @yhwang - does this change preserve backward compatibility?

No, the type field was optional in the old KFP version. However, the upcoming v2 KFP version will make the type required sometimes next year, so we want to update the AIF360 component to be ready for the new KFP versions.

@nrkarthikeyan
Copy link
Collaborator

From your response, it seems like type field is optional now, but will become mandatory later. So including the type field would preserve backward compatibility right? Just making sure it wont break any existing code.

@Tomcli
Copy link
Contributor

Tomcli commented Sep 8, 2021

From your response, it seems like type field is optional now, but will become mandatory later. So including the type field would preserve backward compatibility right? Just making sure it wont break any existing code.

yes it will preserve backward compatibility. Nothing will break from this PR. Sorry I probably didn't interpret your message correctly in my last comment.

@nrkarthikeyan nrkarthikeyan merged commit 0ddf84f into Trusted-AI:master Sep 8, 2021
@nrkarthikeyan
Copy link
Collaborator

Ok approved and merged.

@Tomcli
Copy link
Contributor

Tomcli commented Sep 8, 2021

thanks @nrkarthikeyan

@yhwang yhwang deleted the update-component-yaml branch September 8, 2021 19:38
Illia-Kryvoviaz pushed a commit to Illia-Kryvoviaz/AIF360 that referenced this pull request Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants