/recover endpoint should be synchronous

[meejah]

The /recover endpoint is easier to use if it simply waits for recovery to complete (good or bad) before returning. It should take care to return proper status information too (e.g. errors etc).

[exarkun]

I remembered another reason it wasn't originally synchronous. If it's synchronous then the encouraged usage pattern is "post to this endpoint and wait a while for the response". This pattern doesn't suggest a behavior for "you lost your connection before you got a response". The two obvious possibilities are (a) "start polling the status endpoint if you lose your connection" and (b) "post to this endpoint again to get hooked back up to completion notification".

If it's idempotent and async then the encouraged usage pattern is "post to this endpoint until you get an Accepted response, then poll the status endpoint until you get a success or failure result". This pattern is robust to network failures.

(a) is more work because it just devolves to the idempotent/async pattern and you may as well not bother with the extra work to handle the synchronous result case. (b) is maybe a little better, except you still basically have to implement a poll loop. It might not loop as often, which is nice, but the code complexity is all still there.

This doesn't necessarily mean we shouldn't make the endpoint synchronous but I think it at least means there is some interaction with #397 - we have to make sure there is a distinction between "retry the recovery" and "hook me back up to completion notification for the recovery already started" (for (b)).

And if we eventually want more progress reporting information, client code is going to have to poll anyway.

So it looks to me more like the interaction with #397 means the synchronous version is more complexity rather than less. Thoughts?

[meejah]

Hmmm, right ... and I guess also "what if two clients both hit the endpoint" (although I'd expect that to be "not normal" we don't do anything about it?).

What about:

• there are either zero or one recoveries going on (could be represented internally like self._recovery_d = None or not?)
• when you hit the endpoint, there is either a recovery ongoing or not:
  • if not: start one (and wait)
  • if yes: (also) wait for it to end (i.e. also join _recovery_d approximately)

I think this might be approximately the same code as we really need to support a /wait-for-recovery endpoint as well.

But, maybe we don't want that endpoint at all and just force clients to do it? That is, "they [probably] want to poll status information anyway, so 🤷 make them do it always". It still feels not-great to just have to constantly poll .... maybe the "status" endpoint could take an option which is "the last status I saw" and only return when it changes...?

[meejah]

Then at least the client-polling loop is something like:

status = None
while status not in ("finished", "error"):
    status = await request_the_status_endpoint(status)
    update_ui(status)

[exarkun]

An API like

GET /.../recover?after=<a>

200 OK

{"id": <b>, "stage": ...}

where the response doesn't happen until <b> != <a> and <b> changes whenever the status changes seems doable.

[meejah]

yeah
(don't know what you mean "id" to be used for there, but I was just thinking of using ?stage=X approximately...)

[meejah]

Addressed by #410