add ConnectOptions dataclass #610
Conversation
swelborn
force-pushed
the
connect-api
branch
from
4f267f9
to
a15f7c1
Compare
nats/aio/client.py
Outdated
| @@ -419,6 +457,41 @@ async def subscribe_handler(msg): | |||
| asyncio.run(main()) | |||
|
|
|||
| """ | |||
|
|
|||
| if config: | |||
There was a problem hiding this comment.
Bigger refactor but maybe merge into the config rather than out of it?
| @@ -1261,7 +1334,7 @@ async def _flush_pending( | |||
| except asyncio.CancelledError: | |||
| pass | |||
|
|
|||
| def _setup_server_pool(self, connect_url: Union[List[str]]) -> None: | |||
| def _setup_server_pool(self, connect_url: Union[str, List[str]]) -> None: | |||
There was a problem hiding this comment.
Yes this throws a typing error -- Union[List[str]] is just List[str], and _setup_server_pool accepts both str and List[str] (via isinstance(...).
It is not exactly related, but is in the connect call and should be fixed... do you think it belongs in a separate PR?
There was a problem hiding this comment.
Okay, just checking. I'm doing a lint fix PR so that should take care of it. Can just leave it in for now.
this inspects call signature at runtime to prevent an ugly block of if/else to merge kwargs into config
|
I tried something out with merging. Let me know if this approach will work. It looks at the call signature on every connect() call, so that's not ideal but I don't imagine it would cost a significant performance hit. |
Yeah not worried about connect having overhead. Mostly the hot paths (e.g io where we have to be particularly careful). |
To fix #600.
Let me know if this is an OK approach.
Docstring is incomplete, but not sure where you'd like to put that. I can also add/change some tests, but it would be great if you could point me in the right direction for where to add those (there are 4
test_clientfiles...).