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

Closing SSE Subscriptions #351

Open
riot-jayb opened this issue Oct 30, 2018 · 0 comments
Open

Closing SSE Subscriptions #351

riot-jayb opened this issue Oct 30, 2018 · 0 comments

Comments

@riot-jayb
Copy link

Hello,

We recently noticed when using the event listener functionality that the RemoveEventListener() does not clean up the stream if using SSE mode.

Based on the current code, it seems that this would not be an issue if you (a) reuse the marathon client or (b) use one global SSE listener for all deploys. Unfortunately we are currently recreating marathon client on every deploy, and also trying to subscribe one listener per deploy.

While we have a few options that we control (namely moving towards a global listener or reusing our marathon client), I would think that adding support to RemoveEventListener() to close the stream would be helpful. Do you agree?

If so, I can work on a PR. From a quick pass, it seems reasonable to add an SSE stream close channel to the marathon client struct and send a message down that channel to the stream listening goroutine when RemoveEventListener() is called in SSE mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant