Defer version check until after bootstrap succeeds #1426
Conversation
|
|
||
| Raises: | ||
| NodeNotReadyError (if node_id is provided) | ||
| NoBrokersAvailable (if node_id is None) |
There was a problem hiding this comment.
So what happens if bootstrapping eventually fails? Will these still get raised somewhere else in the stack?
I know there's code in the wild that is expecting these exceptions, as I used it myself when I updated the Datadog kafka consumer lag check to use the async KafkaClient instead of the old SimpleClient before we had the new KafkaAdminClient.
To clarify, I'm not against this change, I just want to understand the new behavior.
|
I like this PR's goal of creating a stronger separation btween the "liveness" check from the "which version" check. |
|
What's needed to push this across the line? |
|
Going to leave this out of 1.4.X patch release as it seems like a largish change. Will land for next release. |
|
Did this land in 2.0? |
Separated from #1411 . This PR moves version checking from
__init__into_bootstrap. This means that if a cluster is down and bootstrapping fails, version checking will not cause an exception to be raised immediately. Instead, we wait until the initial bootstrap succeeds and then run version checks. In order to handle this I've changed the "unset" version fromNoneto(0, 0). This also allows us to simplify version checking code internally.(0, 0)should cause us to use the lowest available version (likely 0.8.2) if needed but still flag that version checking has not yet happened.This change is