search

elyra-ai / elyra

Elyra extends JupyterLab with an AI centric approach.
https://elyra.readthedocs.io/en/stable/
Apache License 2.0
1.86k stars 344 forks source link

feat: allow users to use version suffix pipeline runs #3183

Closed rkpattnaik780 closed 1 year ago

rkpattnaik780 commented 1 year ago

What changes were proposed in this pull request?

Currently, when pipelines are created using elyra from the Notebooks, each additional iteration of the pipeline run creates a new pipeline version identified by timestamp.

The PR will allow user to switch from timestamp identifiers to versions numbers (v1, etc) using an environment variable KFP_SUFFIX_USE_VERSION. This will make the pipeline runs easily distinguishable.

Fixes #3182

How was this pull request tested?

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.
lresende commented 1 year ago

@rkpattnaik780 overall LGTM, but could you mention the new env var in the docs

lresende commented 1 year ago

@rkpattnaik780 could you also rebase the pr to make sure all the tests are passing

rkpattnaik780 commented 1 year ago

In an off-thread discussion, we noticed the current implementation will fail if:

We are trying to find a workaround for this issue. Cc @harshad16

harshad16 commented 1 year ago

Thanks for the pull request @rkpattnaik780 and for the review @lresende due this reason: https://github.com/elyra-ai/elyra/issues/3182#issuecomment-1756077849 we don't require the feature, we have closed the issue. Please feel free to close the PR as well.