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 an error which is introduced by pr7040. #7215

Closed
wants to merge 4 commits into from
Closed

fix an error which is introduced by pr7040. #7215

wants to merge 4 commits into from

Conversation

uuip
Copy link
Contributor

@uuip uuip commented Jan 10, 2022

Description

Nothing to subscribe so should not assign to _pubsub , then drain_events just sleep. The error introduced by pr7040 : RuntimeError: pubsub connection not set: did you forget to call subscribe() or psubscribe()?

discussion

fix an error which is introduced by pr7040.  Nothing to subscribe so should not  assign to _pubsub , then  drain_events  just sleep.    The error   introduced by pr7040 :  `RuntimeError: pubsub connection not set: did you forget to call subscribe() or psubscribe()?`
if nothing to subscribe, don't call redis conn
@lgtm-com
Copy link

lgtm-com bot commented Jan 10, 2022

This pull request introduces 3 alerts when merging 8c2e5a3 into 0620eb2 - view on LGTM.com

new alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Unused import
  • 1 for Module is imported with 'import' and 'import from'

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

could you please fix the test failures?

@auvipy auvipy added this to the 5.2.x milestone Jan 10, 2022
@lgtm-com
Copy link

lgtm-com bot commented Jan 12, 2022

This pull request fixes 2 alerts when merging 71a2e4a into 9377e94 - view on LGTM.com

fixed alerts:

  • 1 for Unused import
  • 1 for Module is imported with 'import' and 'import from'

@auvipy
Copy link
Member

auvipy commented Jan 12, 2022

There seem to be network issues. so i restarted the build. in the mean time, can you add some additional unit tests for the proposed change?

@auvipy auvipy requested a review from a team January 12, 2022 04:00
@lgtm-com
Copy link

lgtm-com bot commented Jan 12, 2022

This pull request fixes 1 alert when merging 10e504a into 9377e94 - view on LGTM.com

fixed alerts:

  • 1 for Module is imported with 'import' and 'import from'

@uuip
Copy link
Contributor Author

uuip commented Jan 12, 2022

@auvipy please reject this pr. and have a look at #7220

@uuip uuip closed this Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants