-
Notifications
You must be signed in to change notification settings - Fork 350
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
Add support for running pipelines on Argo and Tekton #1239
Conversation
Thanks for making a pull request to Elyra! To try out this branch on binder, follow this link: |
KFP servers based on Argo and Tekton are now accepting and running the pipelines, but Tekton deployment is having issues displaying the results due to some metrics related issues (FYI @ptitzler) |
elyra/pipeline/processor_kfp.py
Outdated
# Compile the new pipeline | ||
try: | ||
pipeline_function = lambda: self._cc_pipeline(pipeline, pipeline_name) # nopep8 E731 | ||
kfp.compiler.Compiler().compile(pipeline_function, pipeline_path) | ||
if ('Argo' == engine): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a way this is perhaps not consistent with schema's default, which is Argo
. If, for whatever reason, the runtime configuration does contain an invalid value XYZ
, the logic would run the Tekton compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also inconsistent with the logic in https://github.com/elyra-ai/elyra/blob/bde3b4f3d284407dc0f42a3c80f8983e1066b749/elyra/pipeline/processor_kfp.py#L157, where we test for Tekton
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other values should not really come in as the metadata is validated and invalid ones will not be shown in the user interface....
It appears that export does not yet take into account the engine type if Python DSL is generated. The Jinja template https://github.com/elyra-ai/elyra/blob/master/elyra/templates/kfp_template.jinja2#L43 always uses the Argo compiler (unless there is some other PR that contains additional changes. |
The export has already been updated by @akchinSTC |
Need a separate doc PR to reflect the changes in https://elyra.readthedocs.io/en/latest/user_guide/runtime-conf.html |
Red Hat ODH release 1.0 will be using Kubeflow Pipelines with Tekton support, so we are enabling Elyra to compile pipelines for both Argo (default) or Tekton based on runtime configuration.
Co-authored-by: Kevin Bates <kbates4@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am good with this up to the doc changes
The doc update looks good. I also ran a couple tests without issues. |
Red Hat ODH release 1.0 will be using Kubeflow Pipelines
with Tekton support, so we are enabling Elyra to compile
pipelines for both Argo (default) or Tekton based on
runtime configuration.
Fixes #1222
Developer's Certificate of Origin 1.1