Fix _TYPE_HOSTS to require covariant Sequence #2325
Conversation
| if not isinstance(hosts, (tuple, list)): | ||
| if isinstance(hosts, str): |
There was a problem hiding this comment.
This change is required because hosts is now a Sequence which allows more than just tuple and list. I considered changing the check to isinstance(hosts, collections.abc.Sequence), but it felt wrong as Python embraces duck typing, which means being a subclass of collections.abc.Sequence is not needed to be an actual sequence. (It was much more convincing when it was still in my head! The irony being that we're doing this to fix type hints where all types are very much explicit.)
Anyway, due to the above, I then decided to switch from a denylist to an allowlist, since only two types currently work: str and NodeConfig. Why disallow NodeConfig then? Because the type does not allow it and I'm not expecting anyone to actually be passing hosts=NodeConfig(...) as it currently only works by accident. (Note that hosts=[NodeConfig(...)] still works.)
(cherry picked from commit c5c4df4)
(cherry picked from commit c5c4df4)
Closes #2321