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-6647] Fix to show all artifacts in pipeline topology view #2779

Merged

Conversation

jeff-phillips-18
Copy link
Contributor

@jeff-phillips-18 jeff-phillips-18 commented May 6, 2024

Closes: RHOAIENG-6647

Description

Artifacts with the same name created by different tasks were being thrown out due to duplicate IDs. Update the generation of the artifact nodes to create a unique ID for the artifact based on its parent task's ID.

Update the version of @patternfly/react-topology to 5.4.0-prerelease.1 to pickup a fix for correct placement of spacer nodes.

How Has This Been Tested?

Verified locally that the artifact nodes show up correctly with the correct dependencies.
Added a test for usePipelineTaskTopology to verify the correct number of nodes are returned.

Screen shots

Before

image

After

image

Request review criteria:

  • 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)

Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Tested in UI and walked through code updates with @jeff-phillips-18 this afternoon. LGTM.

Copy link
Member

@DaoDaoNoCode DaoDaoNoCode left a comment

Choose a reason for hiding this comment

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

Tested, works well.
/lgtm

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
/approve

Copy link
Contributor

openshift-ci bot commented May 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DaoDaoNoCode, Gkrumbach07, jenny-s51

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

@openshift-ci openshift-ci bot added the approved label May 8, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit fa06535 into opendatahub-io:main May 8, 2024
6 checks passed
@jeff-phillips-18 jeff-phillips-18 deleted the pipeline-fixes branch May 9, 2024 14:13
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