Skip to content
This repository was archived by the owner on Nov 11, 2019. It is now read-only.

Conversation

@rail
Copy link
Contributor

@rail rail commented Mar 19, 2019

closes #827

@rail rail self-assigned this Mar 19, 2019
@garbas
Copy link
Contributor

garbas commented Mar 19, 2019

😄 i like the draft feature in github

@rail rail requested review from La0 and garbas March 19, 2019 20:40
@rail
Copy link
Contributor Author

rail commented Mar 19, 2019

@La0 @garbas what do you think about this approach?

It doesn't reconnect, but at least it detects the issue and should exit, letting either heroku or kubernetes to restart the container.

We can probably wrap the function somehow so we reconnect, but I failed to do that properly for now. I may try and pursue this at some point.

@rail
Copy link
Contributor Author

rail commented Mar 20, 2019

In eeac03f I added some reconnect logic. it should work (TM), but I'm not sure what would be the best way to test it...

@La0
Copy link
Contributor

La0 commented Mar 20, 2019

You could test it with mitmproxy between you and the pulse service, kill the proxy for a while and restart it: the pulse listener should try to reconnect...

Thanks for this patch, it will be used on pulselistener a lot 👍

@rail
Copy link
Contributor Author

rail commented Mar 20, 2019

I actually have a real local rabbitmq server that I can use, was wondering which service I should try to emulate :)

@rail
Copy link
Contributor Author

rail commented Mar 20, 2019

Let's try something that I know, shipit worker

@rail rail marked this pull request as ready for review March 20, 2019 14:03
@rail
Copy link
Contributor Author

rail commented Mar 20, 2019

It worked fine locally.

  • I tried the shipit worker without rabbitmq running. It keeps retrying until the server comes up and starts consuming messages afterwards.
  • Killing the server keeps the worker running and retrying until the server comes up. The worker reconnects and starts consuming messages as expected.

I think this should just work. We should probably watch the consumers after this merged to production.

@rail rail merged commit d5a87df into mozilla:master Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Try to reconnect automatically to pulse server

3 participants