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

16556/enhancements for wf reports #16682

Conversation

assuntad23
Copy link
Contributor

@assuntad23 assuntad23 commented Sep 12, 2023

Some completed enhancements requested in #16556

  • For: Insert current time as text. Time shows as "16 August 2023 at 06:42", it would be good to specify this is UTC
  • For: Time workflow was invoked. Shows as "2023-08-16T06:33:14.822141". Would be good to format more clearly, and specify as UTC
  • Useful inclusion: workflow version

How to test the changes?

(Select all options that apply)

  • Instructions for manual testing are as follows:
    1. Create a Workflow
    2. In the edit report view, select the option to add Time a Workflow was invoked
    3. View the workflow report
    4. Note that there is a workflow version at the top, below the title
    5. Note that the current time indicates that it is in UTC
    6. Note that the Time a Workflow was invoked is formatted clearly and indicates it is in UTC

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek
Copy link
Member

mvdbeek commented Sep 13, 2023

  • Useful inclusion: workflow version

I would only use the TRS version if it is available. Otherwise this only corresponds to the user's version, which matches nothing. No version here is probably better than the internal version.

@jmchilton
Copy link
Member

I would only use the TRS version if it is available. Otherwise this only corresponds to the user's version, which matches nothing. No version here is probably better than the internal version.

We have a drop down to see old versions of the workflow right? So the number has some value to the end user for tracking down what ran, if the workflow has been updated, etc.. Along those lines - we could display the update time or a link maybe? I don't love displaying just the number... but is it not better than nothing?

Ooooo... maybe just an info statement about whether an updated version of the workflow exists there would be better? ("A newer version of this workflow exists, click here to run it!")

But I guess we should get feedback from the user as to whether they are specifically looking for the TRS version?

@mvdbeek
Copy link
Member

mvdbeek commented Sep 13, 2023

We have a drop down to see old versions of the workflow right?

that's what I added the version for ... i should have called it a checkpoint though :(.

So the number has some value to the end user for tracking down what ran,

I would think the report is mostly for users that did not run the workflow ?

but is it not better than nothing?

I worry about it saying version 2, but it's not clear at all this is version 2 of the very specific stored workflow, causing great confusion

Can we just include the link to run the particular version ?

@jmchilton
Copy link
Member

that's what I added the version for ... i should have called it a checkpoint though :(.

Let's change it then? Would you be okay with exposing the checkpoint if that is what we called it?

I would think the report is mostly for users that did not run the workflow ?

If you pull down a workflow and run it on your data and it has all the results summarized with context and presentation and caveats/imitations/assumptions spelled out - that is pretty useful for the person running the workflow - a lot more useful than just history items IMO. Especially if they are a Galaxy novice and didn't write the workflow. I intended it initially to be 50/50 - but in practice you need to own all the stuff and such - I think Sam's publishing a report to a page usecase is handling the "not for the user that ran the workflow" better and we really should think about these things as useful things for the workflow user (invoker) potentially.

Can we just include the link to run the particular version ?

I like exposing the checkpoint number - but the actual link would be a first priority and more immediately useful thing.

@mvdbeek
Copy link
Member

mvdbeek commented Sep 13, 2023

yes, sounds good.

@davelopez
Copy link
Contributor

FWIW, I think we use revision in pages as a similar concept, instead of version or checkpoint. Just adding my 2 cents 😆

@assuntad23
Copy link
Contributor Author

Alright -- so, to recap here: I should instead provide a link to rerun the workflow with the particular version that the user used.
And potentially rename the attribute of the workflow to revision or checkpoint and then expose that on the page as well. Does that sound correct?

@assuntad23 assuntad23 marked this pull request as ready for review September 28, 2023 01:16
@github-actions github-actions bot added this to the 23.2 milestone Sep 28, 2023
class="float-right markdown-edit mr-2"
role="button"
size="sm"
title="Run Workflow Again"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
title="Run Workflow Again"
title="Run Workflow"

title="Run Workflow Again"
@click="rerunWorkflow()">
Run Again
<FontAwesomeIcon icon="redo" />
Copy link
Member

@mvdbeek mvdbeek Sep 28, 2023

Choose a reason for hiding this comment

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

This isn't a redo as we do for tools, you gotta select all the parameters again.

I'd also move this into its own component and then share it between the different sports where we run workflows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, when the user clicks this button, they should not have to select an input again? They would want it re-run with the input already selected?

Is that what you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd also move this into its own component and then share it between the different sports where we run workflows.

+1 to moving it to it's own component. Are there other places where we already do something like this? Can you give me a pointer? TIA ! :)

Copy link
Member

Choose a reason for hiding this comment

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

We don't do workflow re-runs, no. It's a complicated thing to do that we'll want to do, but re-run implies all the same options are chosen, yes. Keep it simple, just running the workflow is fine.

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

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

Did you check that these appear in the PDF as well ?
I would be in favor of breaking each feature addressed down into a single PR, so we can merge things as they start looking good.

@assuntad23
Copy link
Contributor Author

I've split them off into three PRs so far, since the WF rerun will take a little more work, and will likely still rely on getting the workflowID.
Please check them out (#16757, #16758, and #16760)

@assuntad23
Copy link
Contributor Author

closing this in favor of separate PRs #16757 #16758 #16760

@assuntad23 assuntad23 closed this Oct 9, 2023
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.

4 participants