Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sending requests with reconnected client #343

Merged
merged 1 commit into from
Sep 1, 2022

Conversation

charbonnierg
Copy link
Contributor

This pull request fixes a bug (or at least a non documented behaviour). It brings very little change to the codebase.

Problem description

  • It is not possible to send requests using a client which has been connected twice.

Reproducible example:

from nats import NATS

nc = NATS()

await nc.connect()
await nc.close()
await nc.connect()
# We expect a NoRespondersError but we get a TimeoutError without this PR
await nc.request("some.unknown.subject", timeout=0.1)

Objectives of the PR

  • Let users send requests with clients which were already closed before

Proposal implementation

  • Reset Client._resp_sub_prefix at the end of the Client._close method.

I saw that Client._client_id is already resetted at that time, so I though that it was the right place to reset Client._resp_sub_prefix too.

How was this PR tested ?

I added a single test which is very similar to the example above. If we can talk to NATS server, then a request on a subject without subjecription must raise a NoRespondersError, if not, a TimeoutError is raised and test fails.

@charbonnierg charbonnierg changed the title Support connecting already closed client Support sending requests with reconnected client Sep 1, 2022
@charbonnierg
Copy link
Contributor Author

When I run yapf on my branch, I get a lot of modifications:

        modified:   nats/aio/subscription.py
        modified:   nats/errors.py
        modified:   nats/js/api.py
        modified:   nats/js/client.py
        modified:   nats/js/errors.py
        modified:   nats/js/kv.py
        modified:   nats/js/manager.py
        modified:   nats/nuid.py
        modified:   nats/protocol/parser.py
        modified:   tests/test_client.py
        modified:   tests/test_client_async_await.py
        modified:   tests/test_client_nkeys.py
        modified:   tests/test_client_v2.py
        modified:   tests/test_js.py
        modified:   tests/test_nuid.py
        modified:   tests/test_parser.py
        modified:   tests/utils.py

so I did not run yapf before the commit

Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some clients (like Go) do not expose connect on the instance type to avoid reusing, but in the case of Python this approach would be more convenient since we do allow creating a NATS client instance to connect besides the nats.connect method from the module.

@wallyqs wallyqs merged commit 013d23d into nats-io:main Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants