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.

Sign up for GitHub

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

Jump to bottom

Modify superpipeline params type. #773

Merged
merged 10 commits into from
Nov 7, 2024
Merged

Modify superpipeline params type. #773

merged 10 commits into from
Nov 7, 2024

Conversation

revit13
Copy link
Collaborator

@revit13 revit13 commented Nov 5, 2024 •
edited
Loading

Why are these changes needed?

This PR contains the following changes:

  1. Fixes repo_level_ordering and header_cleanser Makefile such that the CI-CD could compile the pipelines although they are part of the black list whcih means they will not be tested.
  2. All simple KFP pipelines use a dictionary for JSON input parameters, which significantly simplifies editing of the parameters and reduce possible human mistakes. The Super pipelines were not updated according this new convention. The PR fixes it.
  3. Finally, it moves super pipeline examples into the /examples folder, according to [Feature] Move KFP supper pipelines to /examples/kfp #772.

In addition, this PR addresses issue #772

Related issue number (if any).

#772

@revit13 revit13 requested a review from roytman November 5, 2024 12:03
revit13 and others added 2 commits November 5, 2024 14:41
@revit13
Move superworkflows directory.
36f143b 
Signed-off-by: Revital Sur <[email protected]>
@revit13 @roytman
Add transforms to workflow blacklist.
c687c98 
Signed-off-by: Revital Sur <[email protected]>
Co-authored-by: Alexey Roytman <[email protected]>
@revit13 revit13 changed the title Change superpipeline params type. Modify superpipeline params type. Nov 5, 2024
roytman
roytman approved these changes Nov 5, 2024
View reviewed changes
Copy link
Member

@roytman roytman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but let's see reactions on #772

@revit13
Fix license_select in superpipeline.
ef8239f 
Signed-off-by: Revital Sur <[email protected]>
@revit13
Minor change after testing.
720585e 
Signed-off-by: Revital Sur <[email protected]>
@revit13
Minor fix.
ec02ce2 
Signed-off-by: Revital Sur <[email protected]>
@touma-I
Copy link
Collaborator

touma-I commented Nov 5, 2024

I think the reference to #722 is NOT the correct one. Please confirm. thanks

@revit13
Minor fix.
b2ad872 
Signed-off-by: Revital Sur <[email protected]>
@revit13
Copy link
Collaborator Author

revit13 commented Nov 6, 2024 •
edited
Loading

I think the reference to #722 is NOT the correct one. Please confirm. thanks

Yes it was a typo sorry about that. I updated the PR description based on @roytman comments. Thanks

@revit13
Address review comments.
d7b6e01 
Signed-off-by: Revital Sur <[email protected]>
@revit13
Minor fix.
f5ee83f 
Signed-off-by: Revital Sur <[email protected]>
@revit13
Minor fix.
f7c0548 
Signed-off-by: Revital Sur <[email protected]>
@roytman roytman merged commit 0c52678 into IBM:dev Nov 7, 2024
15 checks passed
@roytman roytman mentioned this pull request Nov 7, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roytman roytman roytman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@revit13 @touma-I @roytman