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

feat: Workflow title/description in workflow list view. Fixes #6529 #9805

Conversation

jmeridth
Copy link
Member

@jmeridth jmeridth commented Oct 13, 2022

Screen Shot 2022-10-16 at 10 52 08 PM

Fixes #6529 (partially, no markdown)

  • title and description in list view

TL;DR
Add the following to your workflow annotations to get a title/description to show on the workflow list page:

spec:
  annotations:
    workflows.argoproj.io/title: 'this is a title'
    workflows.argoproj.io/description: 'this is a description'

Signed-off-by: jmeridth jmeridth@gmail.com

Fixes #TODO

Please do not open a pull request until you have checked ALL of these:

  • Create the PR as draft .
  • Run make pre-commit -B to fix codegen and lint problems.
  • Sign-off your commits (otherwise the DCO check will fail).
  • Use a conventional commit message (otherwise the commit message check will fail).
  • "Fixes #" is in both the PR title (for release notes) and this description (to automatically link and close the issue).
  • Add unit or e2e tests. Say how you tested your changes. If you changed the UI, attach screenshots.
  • Github checks are green.
  • Once required tests have passed, mark your PR "Ready for review".

If changes were requested, and you've made them, dismiss the review to get it reviewed again.

@jmeridth
Copy link
Member Author

STIL WIP

Getting the following error do to a collision with react-markdown and swagger-ui-react

Compiled with problems:X

ERROR

./node_modules/swagger-ui-react/swagger-ui-es-bundle-core.js 2:298485
Module parse failed: Unexpected token (2:298485)
File was processed with these loaders:
 * ./node_modules/source-map-loader/dist/cjs.js
You may need an additional loader to handle the result of these loaders.
| /*! For license information please see swagger-ui-es-bundle-core.js.LICENSE.txt */
...

And line 2, character 298485 is a ?? character (nullish coalescing operator).

...
;return e[r]??(e[r]=[]),e[r]
...

I've taken a barebones repo and just added swagger-ui as a dependency and got compilation errors also. Either I find another markdown library or figure out this issue.

Still working through it.

@alexec
Copy link
Contributor

alexec commented Oct 13, 2022

Maybe no markdown in v1? Could be a security risk, allow users to render JavaScript in other users browsers.

@jmeridth
Copy link
Member Author

@alexec that sounds good to me. Will rollback the Markdown. Cheers for the suggestion.

@jmeridth jmeridth force-pushed the jm/workflow-title-description-in-list-view branch 2 times, most recently from be05d8c to 2b81e7b Compare October 17, 2022 04:10
@jmeridth
Copy link
Member Author

Have an issue where, when the list refreshes the first time (based on timer?) it shows the previous workflow.metadata.name until the screen is manually refreshed. The refresh is not including the annotations I believe. Looking where this might be missing.

@stale
Copy link

stale bot commented Nov 1, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

@stale stale bot added the problem/stale This has not had a response in some time label Nov 1, 2022
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Are you still working on this?

@stale stale bot removed the problem/stale This has not had a response in some time label Nov 2, 2022
@jmeridth
Copy link
Member Author

jmeridth commented Nov 7, 2022

One last hurdle this weekend. Should have the PR ready for review in the next 24 hours.

@stale
Copy link

stale bot commented Nov 23, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

@stale stale bot added the problem/stale This has not had a response in some time label Nov 23, 2022
…j#6529

Fixes argoproj#6529

- [x] title and description in list view

Signed-off-by: jmeridth <jmeridth@gmail.com>
@jmeridth jmeridth force-pushed the jm/workflow-title-description-in-list-view branch from 2b81e7b to d28d2ee Compare November 28, 2022 05:06
@jmeridth
Copy link
Member Author

Found the issue with the refresh. Fixed. Waiting on CI and then will mark as ready for review.

@stale stale bot removed problem/stale This has not had a response in some time labels Nov 28, 2022
@jmeridth
Copy link
Member Author

jmeridth commented Nov 28, 2022

@terrytangyuan could you please rerun the cancelled GitHub Action (job)? It timed out after 8mins. I don't think my change would cause that. If that can finish/pass, I'm ready for this change to be reviewed. Cheers.

Master update is doing re-run, hope it gets past the timeout now. 🤞

@@ -90,6 +91,7 @@ export class WorkflowsService {
'result.object.status.progress',
'result.type',
'result.object.metadata.labels',
'result.object.metadata.annotations',
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the part that I was missing on the refreshes. Old name was showing after 5 second watchlist refresh. Adding this keeps the title and description if present in the annotations on refresh.

@jmeridth jmeridth marked this pull request as ready for review November 28, 2022 17:00
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thank you!

@terrytangyuan terrytangyuan merged commit 7c805fe into argoproj:master Nov 28, 2022
@jmeridth jmeridth deleted the jm/workflow-title-description-in-list-view branch November 28, 2022 20:41
jmeridth added a commit to jmeridth/argo-workflows that referenced this pull request Mar 31, 2023
Already working in normal workflow view via argoproj#9805

- create shared react component for workflow row name to dry up usage of this code in both normal and archive workflow lists
- add documentation regarding title and description annotations

Signed-off-by: jmeridth <jmeridth@gmail.com>
jmeridth added a commit to jmeridth/argo-workflows that referenced this pull request Mar 31, 2023
Already working in normal workflow view via argoproj#9805

- create shared react component for workflow row name to dry up usage of this code in both normal and archive workflow lists
- add documentation regarding title and description annotations

Signed-off-by: jmeridth <jmeridth@gmail.com>
jmeridth added a commit to jmeridth/argo-workflows that referenced this pull request Mar 31, 2023
Already working in normal workflow view via argoproj#9805

- create shared react component for workflow row name to dry up usage of this code in both normal and archive workflow lists
- add documentation regarding title and description annotations

Signed-off-by: jmeridth <jmeridth@gmail.com>
jmeridth added a commit to jmeridth/argo-workflows that referenced this pull request Mar 31, 2023
Already working in normal workflow view via argoproj#9805

- create shared react component for workflow row name to dry up usage of this code in both normal and archive workflow lists
- add documentation regarding title and description annotations

Signed-off-by: jmeridth <jmeridth@gmail.com>
jmeridth added a commit to jmeridth/argo-workflows that referenced this pull request Mar 31, 2023
Already working in normal workflow view via argoproj#9805

- create shared react component for workflow row name to dry up usage of this code in both normal and archive workflow lists
- add documentation regarding title and description annotations

Signed-off-by: jmeridth <jmeridth@gmail.com>
jmeridth added a commit to jmeridth/argo-workflows that referenced this pull request Mar 31, 2023
Already working in normal workflow view via argoproj#9805

- create shared react component for workflow row name to dry up usage of this code in both normal and archive workflow lists
- add documentation regarding title and description annotations

Signed-off-by: jmeridth <jmeridth@gmail.com>
tcolgate added a commit to QubitProducts/kube-ci that referenced this pull request Apr 12, 2023
We include the PR URL in a custom annotation, and we set the
argo annotation that allows us to control the Title used in the
argo UI (see argoproj/argo-workflows#9805)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show workflow title/description in UI
4 participants