-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Prevent from subscribing to empty channels #7040
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like the patch, however can you atleast provide minimal unit test?
This pull request introduces 2 alerts when merging ad5dab8 into 5d68d78 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey, thanks for the tests! this pr #7041 recently stabilizes the CI. could you please pull from master and check this still passes the CI?
=========================== short test summary info ============================ |
but e5d9980 is green! |
Still an issue in Sentry 5.2.3 (Python 3.9, redis-server 5.0.4, redis-py 3.5.3) |
can you try #7232 ? |
Note: Before submitting this pull request, please review our contributing
guidelines.
Description
If an error occurs while calling
self._pubsub.get_message
, method_reconnect_pubsub
will be called.after the following loop is completed:
all subscribed tasks might be discarded and the set
self.subscribed_to
will be empty. Thus a call toself._pubsub.subscribe(*self.subscribed_to)
will leads the following error:Maybe related issues:
redis/redis-py#1351
#6993