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

✨ NEW: Expose aio_pika.Connection.add_close_callback #104

Merged
merged 3 commits into from
Mar 3, 2021

Conversation

chrisjsewell
Copy link
Member

Allow RmqThreadCommunicator to be initialised
with a connection_close_callback.

This is an initial step towards addressing aiidateam/aiida-core#4595 (comment).

Note the aio_pika.RobustConnection class also has an add_reconnect_callback method, but this is not necessary to add if we only want to identify closures (a reconnection is triggered after the close_callbacks have been called).

Note also that when these callbacks are called, all exceptions are caught (see https://github.com/mosquito/aio-pika/blob/94066fa900d9c08624d936f9b94037640267ac37/aio_pika/tools.py#L162-L168) which may provide some challenges if the goal is to except the daemon worker 😒

Oh and one last thing, the callback can be added as "weak=True" (i.e. to a WeakSet) which I'm not sure whether is good to use or not (or to add an additional parameter to control it)

Allow `RmqThreadCommunicator` to be initialised
with a connection_close_callback.
@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.34%. Comparing base (cf2e140) to head (803d32b).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #104      +/-   ##
===========================================
+ Coverage    90.29%   90.34%   +0.06%     
===========================================
  Files           16       16              
  Lines         1132     1138       +6     
===========================================
+ Hits          1022     1028       +6     
  Misses         110      110              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@muhrin muhrin left a comment

Choose a reason for hiding this comment

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

Can we change this to be a method e.g. add_close_callback rather than a constructor argument, for a couple of reasons:

  1. The number of constructor arguments is already large and getting difficult to look through
  2. To decouple construction and use of the class such that clients that get passed an instance can also listen for connection closures.

@chrisjsewell
Copy link
Member Author

Can we change this to be a method

ok done

@chrisjsewell chrisjsewell requested a review from muhrin March 2, 2021 21:23
kiwipy/rmq/threadcomms.py Outdated Show resolved Hide resolved
@chrisjsewell chrisjsewell requested a review from muhrin March 3, 2021 15:45
@chrisjsewell chrisjsewell merged commit cfc0498 into develop Mar 3, 2021
@chrisjsewell chrisjsewell deleted the connection_close_callback branch March 3, 2021 15:50
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