search

dapper91 / pjrpc

python json-rpc client/server without boilerplate
https://pjrpc.readthedocs.io
The Unlicense
30 stars 3 forks source link

examples/aio_pika_server.py: Enable logging (and fix the warning) #64

Closed bernhardkaindl closed 2 years ago

bernhardkaindl commented 2 years ago

To show warnings to be fixed in the aio_pika server, enable logging in examples/aio_pika_server.py:

This uncovers a missing await in pjrpc/server/integration/aio_pika.py:

pjrpc/server/integration/aio_pika.py:103: \
RuntimeWarning: coroutine 'IncomingMessage.ack' was never awaited
  message.ack()

I push the fix for the warning in this pull request as a second commit.

codecov-commenter commented 2 years ago

Codecov Report

Merging #64 (21a4367) into dev (b3b7e3b) will increase coverage by 0.03%. The diff coverage is 94.11%.

:exclamation: Current head 21a4367 differs from pull request most recent head 7277097. Consider uploading reports for the commit 7277097 to get more accurate results

@@            Coverage Diff             @@
##              dev      #64      +/-   ##
==========================================
+ Coverage   78.63%   78.67%   +0.03%     
==========================================
  Files          38       38              
  Lines        2373     2377       +4     
==========================================
+ Hits         1866     1870       +4     
  Misses        507      507
Flag Coverage Δ
unittests 78.67% <94.11%> (+0.03%) :arrow_up:

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

Impacted Files Coverage Δ
pjrpc/server/integration/aio_pika.py 0.00% <0.00%> (ø)
pjrpc/__about__.py 100.00% <100.00%> (ø)
pjrpc/client/integrations/pytest.py 93.44% <100.00%> (+0.22%) :arrow_up:
pjrpc/server/dispatcher.py 87.44% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f81af98...7277097. Read the comment docs.

dapper91 commented 2 years ago

@bernhardkaindl Approved. Please rebase to dev before merge.

bernhardkaindl commented 2 years ago

Since there is no conflict, there should be no problem to merge as is, but I push the rebase to current dev now: