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 Workers Connecting to Broker Twice #1620

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

ABrain7710
Copy link
Contributor

@ABrain7710 ABrain7710 commented Feb 9, 2022

Many of the workers have shown in the logs that they connected to the broker twice. While it hasn't seemed to cause any issues, it does not need to be happening. I figured out that the method call for a worker to connect to the broker was being made in the __init__ method of the Worker class and the WorkerGitInterfaceable class. And since the WorkerGitInterfaceable class inherits from the Worker class, any time the WorkerGitInterfaceable class is instantiated, the broker is connected to twice. With a worker this happens because most of them inherit from WorkerGitInterfaceable. As additional proof that this is the issue, you can see that workers (facade worker) that inherit from just the Worker class don't have the issue, but workers (github_worker, pr_worker, gitlab_issues_worker) that inherit from WorkerGitInterfaceable do.

@ABrain7710 ABrain7710 added bug-fix Fixes a bug workers Related to data workers labels Feb 9, 2022
Copy link
Contributor

@IsaacMilarky IsaacMilarky left a comment

Choose a reason for hiding this comment

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

Looks good. I would merge if it runs like normal like this which I'm pretty sure it would

@sgoggins sgoggins merged commit dea48eb into main Feb 14, 2022
@ABrain7710 ABrain7710 deleted the andrew-fix-double-broker-connection branch June 24, 2022 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix Fixes a bug workers Related to data workers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants