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

[RHOAIENG-4702] Improve unique naming for runs and versions in pipelines #3008

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

jpuzz0
Copy link
Contributor

@jpuzz0 jpuzz0 commented Jul 16, 2024

Closes: RHOAIENG-4702

Description

  • Added duplicate name info helper text when uploading a pipeline, version or creating a run or schedule.
  • Fixed issue where if > 20 versions existed within a particular pipeline, only the latest 20 were being used in the list of versions used on every runs table - this would result in pipeline versions not rendered in the table - instead just a hyphen, and consequently effected the list of pipeline versions available for the table filter toolbar dropdown.
  • Fixed duplicate running of ~30 tests that was leading to an unnecessary additional ~2.5 minutes of CI cypress test runtime.
image

How Has This Been Tested?

Manual testing, and updated existing tests to verify the existence of the duplicate name helper text.

Test Impact

Creation of runs, schedules, and importing of pipelines and versions.

How to test

  1. When creating a new run, specify a "Name" of a run that already exists within the project you're in, verify Info helper text exists, with the ability to still create the run, since this is not an API limitation for runs or recurring runs.
  2. Perform step 1, but instead use the "Create schedule" page/form.
  3. Open the import pipeline modal from the pipelines list page, specify a "Name" of a pipeline that already exists within the project you're in, verify helper text exists.
  4. Perform step 3 except from the "Create run" page (accessed from below the Pipeline form dropdown).
  5. Open the import pipeline version modal from the pipelines list page, specify a "Version name" of a pipeline version that already exists within the project you're in and the pipeline specified above it, verify helper text exists.
  6. Perform step 5 except from the "Create run" page (accessed from below the Pipeline version form dropdown).

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change (find relevant UX in the SMEs section).

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

@jpuzz0 jpuzz0 requested review from yannnz and removed request for lucferbux and DaoDaoNoCode July 16, 2024 14:42
@jpuzz0 jpuzz0 force-pushed the RHOAIENG-4702 branch 4 times, most recently from 6c22045 to 462032d Compare July 17, 2024 05:21
@Gkrumbach07
Copy link
Member

Added comments. Biggest thing is that we dont want to be doing a recursive fetching resources. This is the same reason why we dont have the feature for "delete a pipeline and it deletes all the versions too"

@jpuzz0 jpuzz0 force-pushed the RHOAIENG-4702 branch 5 times, most recently from 929be1e to 722e3c5 Compare July 18, 2024 22:02
@jpuzz0 jpuzz0 force-pushed the RHOAIENG-4702 branch 6 times, most recently from 3231065 to d61f557 Compare July 24, 2024 17:37
Copy link
Member

@Gkrumbach07 Gkrumbach07 left a comment

Choose a reason for hiding this comment

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

/lgtm

tested, debounce works, does not block submit

Copy link
Contributor

openshift-ci bot commented Jul 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Gkrumbach07

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

openshift-ci bot commented Jul 24, 2024

New changes are detected. LGTM label has been removed.

@jpuzz0 jpuzz0 added the lgtm label Jul 24, 2024
@jpuzz0 jpuzz0 merged commit 7332e85 into opendatahub-io:main Jul 24, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants