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

KFPv2 support step 1 #226

Merged
merged 68 commits into from
Jun 8, 2024
Merged

KFPv2 support step 1 #226

merged 68 commits into from
Jun 8, 2024

Conversation

roytman
Copy link
Member

@roytman roytman commented Jun 4, 2024

Why are these changes needed?

This is the first step to support KFPv2.
Here we provide shared libraries and pipelines, which can work with KFPv1 and KFPv2.
We have tested the LFPv1 option, KFPv2 was only partially tested.
Next steps are:

  • tests ALL KFPv2 pipelines
  • update documentation
  • work on KFPv2 super pipelines.

Related issue number (if any).

revit13 and others added 25 commits June 2, 2024 10:40
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
@roytman roytman marked this pull request as draft June 4, 2024 09:06
revit13 and others added 4 commits June 4, 2024 13:58
revit13 and others added 19 commits June 5, 2024 22:39
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
@roytman roytman changed the title KFPv2 support KFPv2 support step 1 Jun 7, 2024
@roytman roytman marked this pull request as ready for review June 7, 2024 05:00
@roytman roytman requested a review from blublinsky June 7, 2024 05:00
Copy link
Collaborator

@blublinsky blublinsky left a comment

Choose a reason for hiding this comment

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

This is good, but I think it needs more work

kfp/kfp_support_lib/Makefile Show resolved Hide resolved
logger.warning("All executions failed")
sys.exit(1)
else:
logger.info(f"{n_launches} ot of {len(config_value)} succeeded")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here. It does not depend on KFP version

# wait for completion
status, error = utils.wait_pipeline_completion(run_id=run, wait=10)
assert status.lower() == "succeeded"
assert error == ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

transforms/universal/filter/src/local_pipeline.py Outdated Show resolved Hide resolved
@blublinsky
Copy link
Collaborator

It looks like there is more make stuff then the actual code

@roytman
Copy link
Member Author

roytman commented Jun 7, 2024

It looks like there is more make stuff then the actual code

This is the beauty, we can reuse the same pipelines for both versions.

Signed-off-by: Alexey Roytman <[email protected]>
Signed-off-by: Alexey Roytman <[email protected]>
Copy link
Collaborator

@blublinsky blublinsky left a comment

Choose a reason for hiding this comment

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

This is good, but I think it needs more work

@roytman roytman merged commit 36d4982 into dev Jun 8, 2024
15 checks passed
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.

4 participants