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

add links for skipped tasks in voxelytics workflow reports #8006

Merged
merged 7 commits into from
Aug 20, 2024

Conversation

normanrz
Copy link
Member

@normanrz normanrz commented Aug 16, 2024

This PR (finally) adds links for skipped tasks in the workflow reports to the workflows/runs where the artifacts were produced. The artifact information is also shown inline.
Screenshot 2024-08-16 at 21 30 36

Steps to test:

  • Get 2 workflows, 1 that has just the first few tasks and a second that appends more tasks
  • Run the first workflow; then, run the second workflow skipping the existing tasks
  • Look at the workflow report of the second workflow

Issues:


@normanrz normanrz requested review from fm3 and daniel-wer August 16, 2024 19:33
@normanrz normanrz self-assigned this Aug 16, 2024
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Looking good already :)

FROM webknossos.voxelytics_artifacts a
JOIN latest_complete_tasks t ON t._id = a._task
""".as[(String, String, String, String, Long, Long, String, String, String)])
LEFT JOIN ( -- when the task is skipped, the artifact from the producing task run is joined for linking
Copy link
Member

Choose a reason for hiding this comment

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

please add the discussed distinct constraint to the schema, and a comment here explaining how the fanout effect is prevented (joining at most one item, due to the distinct constraint)

Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Works for me and backend looks good :)
Leaving final approval for @daniel-wer

Maybe a tooltip on the links to the “foreign workflow” would be nice, but not super important

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Spectacular 🎉 This already made my week and is going to be very helpful in the future :)

Should I retest this somehow or do you think it's fine to test this in prod using the real-world reports?

How does this affect the file size shown next to a workflow in the workflow list? Same for the "Show Disk Usage of Artifacts" modal that can be displayed from within a report? I don't have a strong opinion on which way it should be counted, but I'd like to at least document the current behavior in this PR.

@normanrz
Copy link
Member Author

How does this affect the file size shown next to a workflow in the workflow list? Same for the "Show Disk Usage of Artifacts" modal that can be displayed from within a report? I don't have a strong opinion on which way it should be counted, but I'd like to at least document the current behavior in this PR.

The storage display in the workflow list is not affected by this change. I think that makes sense. Only artifacts that are produced by that particular workflow are counted.

The "Show Disk Usage of Artifacts" would show include the foreign artifacts. Should I filter them out for consistency?

@daniel-wer
Copy link
Member

The "Show Disk Usage of Artifacts" would show include the foreign artifacts. Should I filter them out for consistency?

I'd say it's ok as it is

@normanrz normanrz merged commit 449ec25 into master Aug 20, 2024
2 checks passed
@normanrz normanrz deleted the skipped-artifacts branch August 20, 2024 11:54
@fm3 fm3 mentioned this pull request Aug 20, 2024
16 tasks
knollengewaechs pushed a commit that referenced this pull request Sep 2, 2024
* add links for skipped tasks in voxelytics workflow reports

* changelog

* comments

* styling

* add evolution and comment

---------

Co-authored-by: Florian M <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display existing artifacts for skipped tasks in voxelytics reports
3 participants