-
Notifications
You must be signed in to change notification settings - Fork 349
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
Migrate Elyra to KFP v2 (cont.) #3273
base: main
Are you sure you want to change the base?
Conversation
@@ -730,7 +709,7 @@ | |||
pipeline.pipeline_properties.get(pipeline_constants.COS_OBJECT_PREFIX), pipeline_instance_id | |||
) | |||
# - load the generic component definition template | |||
template_env = Environment(loader=PackageLoader("elyra", "templates/kubeflow/v1")) | |||
template_env = Environment(loader=PackageLoader("elyra", "templates/kubeflow/v2")) |
Check warning
Code scanning / CodeQL
Jinja2 templating with autoescape=False Medium
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.
lgtm
just few details:
- we removed
kfp-tekton
in initial work as it only support kfp 1.x , not kfp 2.x.
The project: https://github.com/kubeflow/kfp-tekton/releases, is updating time to time, so in case someone wants to includetekton
support, would need to check and accommodate it. As kfp itself is aligning onargo
, perhaps going forward with that way be the best, based on my observation. - Some changes are explicit of the ODH.
Thanks for the review, @harshad16! I'll look into the points you've raised. |
Could we remove the tekton options and docs as well ? |
Signed-off-by: Harshad Reddy Nalla <[email protected]>
Signed-off-by: Guilherme Caponetto <[email protected]>
Signed-off-by: Guilherme Caponetto <[email protected]>
Signed-off-by: Guilherme Caponetto <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3273 +/- ##
=======================================
Coverage ? 82.83%
=======================================
Files ? 161
Lines ? 19830
Branches ? 509
=======================================
Hits ? 16426
Misses ? 3220
Partials ? 184 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Guilherme Caponetto <[email protected]>
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.
/lgtm
What changes were proposed in this pull request?
Moving forward with the work started in #3219
Developer's Certificate of Origin 1.1