-
Notifications
You must be signed in to change notification settings - Fork 955
Remove redis connection from top level #3218
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
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.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
pylint
tests/test_tasks/test_task_utlities/test_redis/test_redis_list.py|64 col 27| W0621: Redefining name 'redis_list' from outer scope (line 8) (redefined-outer-name)
tests/test_tasks/test_task_utlities/test_redis/test_redis_list.py|70 col 4| C0200: Consider using enumerate instead of iterating with range and len (consider-using-enumerate)
tests/test_tasks/test_task_utlities/test_key_handler/test_github_api_key_handler.py
Show resolved
Hide resolved
Ulincsys
left a comment
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.
LGTM
|
@JohnStrunk / @MoralCode : The end to end test is timing out, but not "failing" in the ordinary sense. . I am wonder if the timeout is a parameter we need to set higher? |
This isnt a timeout issue, the code in this PR made a change that broke something. The stacktrace above the timeout error points to: This was also flagged by a bot that is installed https://github.com/chaoss/augur/pull/3218/files#r2191362618 (among many other warnings) |
MoralCode
left a comment
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'd recommend holding off until the various warnings and errors caught by the automation tools (the bot and the E2E CI job) are resolved
|
|
||
| self.id = getpid() | ||
| self.stdin: PubSub = conn.pubsub(ignore_subscribe_messages = True) | ||
| self.stdin: PubSub = self.conn.pubsub(ignore_subscribe_messages = True) |
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 think this needs to be fixed.
Update to address error message in end to end build. Signed-off-by: Sean P. Goggins <outdoors@acm.org>
sgoggins
left a comment
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.
LGTM!
Ulincsys
left a comment
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.
LGMT
Description
This PR fixes #
#3047
Signed commits