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

Adds support for workflow node aliasing via identifier field #10592

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

mabashian
Copy link
Member

SUMMARY

link #2032

cc @AlanCoding since you may have opinions here based on your comments in the original issue.

The implementation leverages the identifier field on a workflow node in order to allow users to specify an alias to be displayed on the node instead of the resource name. This would allow users to uniquely identify nodes that have the same underlying unified job template. If the string is a uuid (generated by the server) then we still display the resource name.

Here it is in action:

node_alias

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • UI

@mabashian
Copy link
Member Author

cc @wenottingham - if this is a bad idea let me know and we'll close this one

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@wenottingham
Copy link
Contributor

@mabashian no, this is neat. Is it plumbed through the job detail screen as well if you cllick into it on a workflow run?

@AlanCoding
Copy link
Member

If the string is a uuid (generated by the server) then we still display the resource name.

That's pretty critical behavior. I think I agree with your decisions here.

Disclosure: I haven't actually looked at the code yet.

How are you distinguishing UUID type identifiers from user-given ones? Some pattern matching?

@mabashian
Copy link
Member Author

How are you distinguishing UUID type identifiers from user-given ones? Some pattern matching?

yea right now it's just a regex match

@mabashian
Copy link
Member Author

Is it plumbed through the job detail screen as well if you cllick into it on a workflow run?

I'll have to double check on this one

@mabashian mabashian force-pushed the 2032-workflow-node-alias branch from 9132289 to b591747 Compare July 9, 2021 20:16
@mabashian
Copy link
Member Author

@wenottingham so if you click on a running node to view the details I don't know that we have the info we need via the api to display the identifier. We'd need something that pointed back to the node in the summary_fields for the job. Maybe @AlanCoding knows if that's possible or not. If it was there we could certainly display it in the details.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@AlanCoding
Copy link
Member

We already show the workflow job in the list view. I'd bet this query is poor performance, and I see no UI need for it in the list view. I digress.

We fully load the job's node before we can load the workflow. So as long as we are properly using the cache (we likely aren't), there is no performance impact for giving node details. identifier is a direct node field, so no worries there.

Answering the question, no technical blockers, but we should subject the view's performance to scrutiny (using the normal Django toolbar stuff) as we implement this.

@AlexSCorey AlexSCorey self-requested a review July 13, 2021 19:05
@keithjgrant keithjgrant self-requested a review July 19, 2021 20:42
@mabashian mabashian force-pushed the 2032-workflow-node-alias branch 2 times, most recently from 938c871 to e4a5023 Compare July 22, 2021 18:05
@mabashian mabashian force-pushed the 2032-workflow-node-alias branch from e4a5023 to a5f5570 Compare July 27, 2021 13:03
@mabashian mabashian force-pushed the 2032-workflow-node-alias branch from a5f5570 to 9a39ff1 Compare July 30, 2021 13:46
@tiagodread tiagodread self-assigned this Aug 2, 2021
@tiagodread
Copy link
Contributor

The Node Alias is not available on job details but this is really cool, on hover over the node the user can see the Node Alias in addition to Resource Name

Screenshot from 2021-08-03 13-16-11

Copy link
Contributor

@tiagodread tiagodread left a comment

Choose a reason for hiding this comment

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

@mabashian once the rebase is done we can merge this one

@mabashian mabashian force-pushed the 2032-workflow-node-alias branch from 9a39ff1 to 0213fb5 Compare August 3, 2021 18:08
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.

5 participants