-
-
Notifications
You must be signed in to change notification settings - Fork 748
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
Ignore MemberAlreadyExist exception during startup. #5765
base: master
Are you sure you want to change the base?
Ignore MemberAlreadyExist exception during startup. #5765
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.
Looks good. Needs a changelog entry, and one of your commits is using an email address that is not attached to your github account, so you'll need to rewrite the commits to fix that (or the CLA bot will keep complaining).
Also, I wonder if there's a decent way to add a test for this. Can you look into that?
81625e5
to
0261c73
Compare
@khushboobhatia01 I rebased and fixed the author email. Now the CLA bot is not complaining. |
try: | ||
return coordinator.join_group(group_id, capabilities=capabilities).get() | ||
except MemberAlreadyExist: | ||
pass |
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.
More comment on legacy, but in the try we return whatever coordinator.join_group returns, but if memberalreadyexists we don't return anything...
I think in fact the join_group doesn't return anything - so probably just worth changing it so that we change the try to remove the "return" on the call to coordinator.join_group. Or if its expected to return something then we need to adjust the except clause...
During service startup, if service registry is enabled we register the service.
If the service member ID already exists in the registry, the service start up fails with an exception MemberAlreadyExist.
This change will ignore this exception and continue with service startup.
Why do we need this change?
Consider a scenario where pod with a Stackstorm service is running.