Arvados

Zoë Ma wrote in #note-7:

23214-cluster-activity-report-cwl-tool-date @ 63967167eae6375b2fa39b1438bfedb1ff1e04de

Everything you wrote here is good. The code is good, it meets the ticket specs, the tests are good, everything is good to merge.

I noticed something while I was reviewing the test runs that I didn't anticipate. I want to think through what it means for larger system usability, because I actually don't know the best way to deal with it. I'm sorry I didn't think of this when I wrote the ticket. We're learning together.

When you run the workflow with the default date, Arvados records the reporting_end input as empty. Which is fair enough. But this means if you wait a few days, then run the workflow again with the same reporting_start and empty reporting_end, Arvados will just reuse the previous run. Instead of doing what you want, which is running the workflow again with today's reporting_end.

Ideally what would happen is today's date would get recorded as the reporting_end input when we submit the workflow. Unfortunately I'm not sure that's possible within our current systems. CWL inputs have a default field, but their types are only File | Directory | Any, so I'm not sure expressions get evaluated here. In the worst case, putting the default expression here doesn't work at all. In the slightly better case, the expression gets recorded as the reporting_end and it gets evaluated at runtime just like it does now. But that's not really an improvement from where we are now: it means the expression would get recorded as the reporting_end in Arvados and the problem of inappropriate reuse would still exist.

A better solution might be to use the WorkReuse hint. This does support expressions, so I think we could say something like (not tested):

WorkReuse:
  enableReuse: "$(!!inputs.reporting_end)" 

And this way you could still reuse reports with set end dates, but Arvados would not try to reuse reports that rely on the implicit end date of today. It would fail to reuse reports run with an empty reporting_end on the same day, but I think that's a less common case and a better problem to have overall.

Would you be okay with experimenting a little bit and seeing:

• Is it possible to record today's date as the workflow's reporting_end input?
• If not, can we dynamically disable reuse along the lines shown above?

And then push to the branch for review the first option that works? Let me know if you have any questions, other ideas, or concerns about this. Thanks.

Sure, I can definitely experiment with this

I have the following observations:

1. The default field to a CWL input parameter doesn't support expressions; the expression is used as text rather than evaluated.
2. I tried also the WorkReuse hint directive as you suggested, but to my surprise, although the CWL file validates, when run from the WB panel it doesn't do what I expect (the subprocess still gets reused, if the other input parameter values are the same but the reporting_end parameter is left null). This may be an issue worthy of further investigation in another ticket (I can try to provide a minimal example). I also tried removing the double quotes around the expression $(!!inputs.reporting_end) in the CWL file but the result was the same.

Maybe we could simply mark the entire CWL tool description as non-reusable and document the reason briefly? This should "work" in the sense of preventing false reuses, at the (small?) cost of losing the certainty of reproducibility for past time intervals (even though I expect that the report will just produce the same output anyway).

In addition, is there any reason we won't capture the cost report CSV file in the outputs, while only capturing the HTML report?

Zoë Ma wrote in #note-10:

The default field to a CWL input parameter doesn't support expressions; the expression is used as text rather than evaluated.

That's what I expected based on my reading of the spec. Thanks for trying anyway.

I tried also the WorkReuse hint directive as you suggested, but to my surprise, although the CWL file validates, when run from the WB panel it doesn't do what I expect (the subprocess still gets reused, if the other input parameter values are the same but the reporting_end parameter is left null). This may be an issue worthy of further investigation in another ticket (I can try to provide a minimal example). I also tried removing the double quotes around the expression $(!!inputs.reporting_end) in the CWL file but the result was the same.

So, like I said, my JavaScript was completely untested and might be wrong. If you want to try out other approaches, feel free to do so.

If you have no luck or just don't want to, yes, please file a separate issue about this. This sounds like a bug… somewhere, but hard to say where off-hand. It would help to attach the CWL and the full container request API records for both arvados-cwl-runner and arv-cluster-activity.

Maybe we could simply mark the entire CWL tool description as non-reusable and document the reason briefly?

I think this is a fine compromise. I think the documentation can even just be a comment. I think users running reports that they actually intend to reuse will be the less common case, and re-running it is cheap relative to a lot of our users' workflows. It's less than ideal but it's not a big deal.

23214-cluster-activity-report-cwl-tool-date @ 33b83e0c5e33ddb64ce691c85eb525856a6735e5

• All agreed upon points are implemented / addressed. Describe changes from pre-implementation design.
  • Implemented the "disable reuse altogether" solution as a simple compromise that doesn't affect actual outcome of running the tool.
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • Apparent inability to correctly reckon with an expression-based enableReuse value will be investigated as a separate issue.
• Code is tested and passing, both automated and manual, what manual testing was done is described.
  • Manually tested on local cluster for no-reuse if run with the same input.
• The tested code incorporates recent main branch changes.
  • Yes
• New or changed UI/UX has gotten feedback from stakeholders.
  • N/A
• Documentation has been updated.
  • The rationale for no-reuse is in the comments in the CWL file.
• Behaves appropriately at the intended scale (describe intended scale).
  • N/A
• Considered backwards and forwards compatibility issues between client and server.
  • N/A
• Follows our coding standards and GUI style guidelines.
  • Yes

Zoë Ma wrote in #note-13:

23214-cluster-activity-report-cwl-tool-date @ 33b83e0c5e33ddb64ce691c85eb525856a6735e5

Thanks. I have gone ahead and merged this branch. We should not close this ticket until the follow-up ticket about investigating WorkReuse expressions has been created. (That's why it's a checklist item, after all, to help ensure we don't forget these things.)

New issue created for the CWL reuse issue as Bug #23227: In a CWL tool description, when enableReuse is an expression, it is not evaluated