Wait for runtime to be ready in __init__

[rbren]

Short description of the problem this fixes or functionality that this introduces. This may be used for the CHANGELOG

I fixed this a few days ago, but my fix seems to have disappeared.

---

Give a summary of what the PR does, explaining any non-trivial design decisions

• Wait for client in init
• Refactor the wait_for_client function so that log refreshing is done separately, making it easier for other callers to use

---

Link of any specific issues this addresses

[tofarr]

Looks like the call to _wait_until_alive was removed in this commit: a45b20a#diff-a7ff0449d091e3e762a4a8b604fe16a3a5570448751817a5a3e207ab18ec648c

I'd like to know if the removal was accidental before putting this back in. @tobitege - any insights?

[tobitege]

Looks like the call to _wait_until_alive was removed in this commit: a45b20a#diff-a7ff0449d091e3e762a4a8b604fe16a3a5570448751817a5a3e207ab18ec648c

I'd like to know if the removal was accidental before putting this back in. @tobitege - any insights?

Those 2 commits are not removing your call, but comment out a log refresh in run_action from a previous PR.

[tobitege]

Line 162 adding back makes sense. If I removed it, then by accident.
But the other changes that process the logs should not just be changed like that, they're there to wait for the client become ready on first call.

[rbren]

But since it's alive in __init__, can't we assume it's alive everywhere else?

[tobitege]

But since it's alive in __init__, can't we assume it's alive everywhere else?

Still, the very first call of _wait_until_alive because of potential timing issues should make sure it got'em all and that extra check is only done once.

The websocket becomes alive but we have to wait for the client to be ready (bash/ipython) before we can start sending commands.

[tobitege]

in #3962 I just tested with minimal reverts, that sudo works again

[rbren]

@tobitege sorry can you explain more? I assume you're correct given that the tests are failing, but once /alive returns 200, can't we assume it's alive from that point forward?

[tobitege]

@tobitege sorry can you explain more? I assume you're correct given that the tests are failing, but once /alive returns 200, can't we assume it's alive from that point forward?

Oops, I had added this to my previous comment instead of making a new one:
The websocket becomes alive but we have to wait for the client to be ready (bash/ipython) before we can start sending commands.

[rbren]

Thanks for the explanation in Slack Tobi!

I think I have a better fix now--_wait_until_alive will now always wait for log_buffer.client_ready. We also reuse the same retry logic instead of having separate retry logic for the log checker

[tobitege]

Thanks for the explanation in Slack Tobi!

I think I have a better fix now--_wait_until_alive will now always wait for log_buffer.client_ready. We also reuse the same retry logic instead of having separate retry logic for the log checker

Yes, looks much cleaner!

[rbren]

OK @tobitege I think the tests should finally be passing

[tobitege]

OK @tobitege I think the tests should finally be passing

Ok. But why move the wait before the super.__init__ in the latest commit? That is doing the bash/ipython env initializations.

[tobitege]

What I read from the logs, the move above the super init isn't helping, other than it will "waste" the first 2 attempts in which guarantueed nothing gets initialized.
It might be more of a time saver to have the retry run with a higher number of retries, but with e.g. a fixed wait time of like 4 seconds, not up to 20.
That would give it a more granular hit-testing, imho.

[rbren]

The super init sets env vars, so it needs the environment to be ready. Otherwise the tests fail

[tobitege]

The super init sets env vars, so it needs the environment to be ready. Otherwise the tests fail

Hmm ok, due to the @retry "polling" effect now.

Any thoughts on the wait times, though?

[rbren]

OK I refactored a bit so that the env is set later on--hopefully this works

[tobitege]

LGTM