-
Notifications
You must be signed in to change notification settings - Fork 233
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
Add connection open/close callbacks #552
base: master
Are you sure you want to change the base?
Conversation
4255bea
to
d095ec5
Compare
Codecov Report
@@ Coverage Diff @@
## master #552 +/- ##
===========================================
- Coverage 98.42% 24.74% -73.68%
===========================================
Files 31 31
Lines 5017 5035 +18
===========================================
- Hits 4938 1246 -3692
- Misses 79 3789 +3710
Continue to review full report at Codecov.
|
e9adf79
to
61bfdf9
Compare
61bfdf9
to
1334b5d
Compare
Good day, thanks for looking to contribute. Can you explain the use case, where you need to have this callback? Maybe there's a more generic solution for those cases. |
Hi @tvoinarovskyi the purpose is to provide the calling application with connection status change indication, in order to allow it to take more actions if needed (such as, but not limited to, event log for instance). I have look thoroughly into the code and could not figure out an elegant way to do that. Callbacks, as suggested, would be a very nice and elegant way to do that. I've tried to write tests for it but figured it would be hard to simulate a change in connection status, and mocking it would be simply not interesting enough. It would be grate to have your opinion! |
I would prefer some reusable generic interface for similar cases. In java client they have this concept of Interceptors. I would consider something similar instead of adding callbacks for all cases. |
There are multiple ways to achieve that. Could you elaborate? |
Sorry 😢 |
|
What do these changes do?
Add new callbacks for connection opened/close for consumer and producer
Are there changes in behavior for the user?
User may add callbacks if they want to be notified for changes in connection status (and take action accordingly)
Related issue number
No
Checklist
CHANGES
folder<issue_id>.<type>
(e.g.588.bugfix
)issue_id
change it to the pr id after creating the PR.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.Fix issue with non-ascii contents in doctest text files.