Add support for pause and resume operations

[ada-globus]

Adds support for pausing and resuming timers, and (drafting off the prior art in flows run resume) inspection of the user's consent forest to determine if required consents are present.

[sirosen]

Should we perhaps add a dedicated option for this, rather than deducing it from --skip-inactive-reason-check? e.g. --update-credentials/--no-update-credentials?

I know we want a more nuanced default behavior for the service (in which case we shouldn't ever have to / want to pass this), but I'm trying to reason through what the CLI behavior will look like absent that and once it's introduced.
I get the connection you're drawing here -- if you pass --skip-inactive-reason-check, it's very unlikely that you do want us to update creds and skip the check -- but is the situation improved by having a dedicated flag?

Today, --no/-update-credentials would give explicit control with a default of omission (meaning "service default"). If the service adds the better default behavior, we naturally inherit it. Yes, it's slightly more awkward to use correctly today, but it means not having to come back to this to "cleanup" when the service changes.

[ada-globus]

1. Agree that at some point we will want to switch to explicit credential update flags and to remove the local check entirely in favor of the service telling us directly.
2. I don't have a super strong POV, so I think I can be easily persuaded if you do.
3. I think the combination of the flags could be a little weird. Should --no-update-credentials imply --skip-inactive-reason-check? Or would you need to specify both if you didn't want that behavior? I think the former maybe should imply the latter from a UX POV, or otherwise we're asking users to understand the internal details of the code path that it's walking. But maybe I am worrying too much and the whole thing is sort of "advanced" behavior anyway that we shouldn't expect most users to think about or use—though in that case we might want to tell them as much.

(Update: The more I am thinking about it, the more I am inclined to add them all for now as you are suggesting and have no hierarchical relationship b/w them, and then we remove the skip option as soon as it's unnecessary and we'll revert to better clarity automatically... I think I'm talking myself into it, even though the state is a little muddier today.)

[sirosen]

Circling back on this over after the weekend, here's what I think we should do, and why:

What? Make update_credentials match the (desired) future server-default behavior, true if there is a GARE and false otherwise.

• That means we always do gare = _get_inactive_reason(job_doc), even outside of the inactive reason check
• Users cannot specify update_credentials via a CLI option (yet?)
• skip_inactive_reason_check is not involved in this process at all -- it is fully orthogonal to this behavior

Why? Consistent behavior across future upgrades and across CLI commands.

• When the service adds this as the default behavior (or something close to this), we can drop the parameter from the CLI and the behaviors will remain self-consistent.
• --skip-inactive-reason-check therefore only controls one thing, which is whether or not the command will refuse to run if there's an unsatisfied GARE detected. That matches the behavior for Flows pretty closely.
• Adding any explicit control for this with --update-credentials puts us in a worse position vis-a-vis usability. We already know what the "best" default behavior is within the current paradigm of behavior; this is just pushing it into the client (temporarily) while we get it into the service.

[ada-globus]

Lol, I just finished going the exact opposite direction. Reading this and digesting.

[sirosen]

Sorry! 😿 ¹

Still, I think most of your testing work would be portable to this approach as well. i.e. The facade will change if you buy my reasoning, but the guts will stay the same.

Footnotes

1. I do not like how GitHub's cat emojis look. Will not use again. ↩

[ada-globus]

All good :)

I backed out all of the changes. --skip-inactive-reason-check now merely suppresses the error, and update_credentials is explicitly true when a GARE is present. Realizing now that in a slight deviation from your suggestion, when a GARE is not present, I have it pass None which will fall back to the service's default of False. I modestly prefer it to defer to the service (which would allow the service to impose additional conditions that might update the credentials without needing to change the CLI first) but functionally this doesn't make a material difference afaict today, so I don't care all that much. I can change it to explicitly send False if you have a stronger preference than I.

[sirosen]

Nah, I'm happy with that course of action. I'll double check the changes and probably approve momentarily!