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

Fix AssertionErrors in close() of aio_pika client. #92

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

bernhardkaindl
Copy link
Contributor

Issue #91 shows an AssertionError when a connected aio_pika client which didn't need to open a result queue calls client.close().

It should be possible to call client.close() also in this case to close the connection to the RabbitMQ server, because when either the client was interrupted, had nothing to do before closing, it, this is the normal state.

Also replace the other assertions in the aio_pika client's close() with simple checks which don't abort the function to close any other remaining connection elements or futures which are not properly handled when simply aborting with an assertion.

For details and a test case, see issue #91.

Issue #91 shows an AssertionError when a connected aio_pika client
which didn't need to open a result queue calls client.close().

It should be possible to call client.close() also in this case
to close the connection to the RabbitMQ server, because when
either the client was interrupted, had nothing to do before
closing, it, this is the normal state.

Also replace the other assertions in the aio_pika client's close()
with simple checks which don't abort the function to close any
other remaining connection elements or futures which are not
properly handled when simply aborting with an assertion.

For details and a test case, see issue #91.
@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2022

Codecov Report

Merging #92 (415416d) into dev (064c5c6) will decrease coverage by 0.05%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##              dev      #92      +/-   ##
==========================================
- Coverage   79.49%   79.44%   -0.06%     
==========================================
  Files          41       40       -1     
  Lines        2717     2715       -2     
==========================================
- Hits         2160     2157       -3     
- Misses        557      558       +1     
Flag Coverage Δ
unittests 79.44% <93.75%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pjrpc/client/backend/aio_pika.py 0.00% <0.00%> (ø)
pjrpc/client/backend/kombu.py 0.00% <0.00%> (ø)
pjrpc/__about__.py 100.00% <100.00%> (ø)
pjrpc/client/client.py 97.55% <100.00%> (ø)
pjrpc/client/integrations/pytest.py 91.85% <100.00%> (ø)
pjrpc/common/__init__.py 91.66% <100.00%> (ø)
pjrpc/common/common.py 85.71% <100.00%> (-2.75%) ⬇️
pjrpc/common/exceptions.py 98.64% <100.00%> (ø)
pjrpc/common/v20.py 91.22% <100.00%> (ø)
pjrpc/server/dispatcher.py 87.73% <100.00%> (+0.04%) ⬆️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bernhardkaindl bernhardkaindl changed the title Fix aio_pika client to never throw AssertionErrors Fix close() of aio_pika client to never throw AssertionErrors Oct 5, 2022
@bernhardkaindl bernhardkaindl changed the title Fix close() of aio_pika client to never throw AssertionErrors Fix AssertionErrors in close() of aio_pika client. Oct 5, 2022
@bernhardkaindl
Copy link
Contributor Author

@dapper91 ping

@bernhardkaindl bernhardkaindl merged commit bd651bd into dev Nov 14, 2022
@dapper91 dapper91 deleted the pika-close-without-assertions branch August 10, 2023 15:12
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