-
Notifications
You must be signed in to change notification settings - Fork 392
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
eventsource continues launching more containers if left open #317
Comments
SamLau95
added a commit
to SamLau95/nbinteract
that referenced
this issue
Feb 9, 2018
jupyterhub/binderhub#317 points out that an EventSource will repeatedly launch servers if it isn't closed properly. Although we close the EventSource after the server launches successfully, I noticed that the EventSource makes repeated requests if the server fails to launch. To test this, I opened a page that uses `nbinteract` to start a currently banned image (`SamLau95/nbinteract-image`). In the console logs, I saw the EventSource endlessly attempting to start a Binder server and verified that the page made multiple requests in the Network tab of the dev tools. To fix this, I enforce a maximum number of failed connection attempts and close the EventSource if it fails to connect after three tries. To test this, I opened the same page as above and verified that the page only makes three attempts to connect before logging an error message and stopping. This should help with #34, although I'm not sure if this was the reason for the Binder overload.
I think we just got burned by this with the nbinteract repository. @rgbkrk did you figure out a way to work around this from your side of things? |
Yeah, we |
Is this still a problem? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If you leave the eventsource open after "ready" it continues launching more
Reproduction steps:
Not a crazy deal for me, I just happened to notice while hitting the event source API directly. My current code does a
.close
appropriately. Figured this should probably be cleaned up though.The text was updated successfully, but these errors were encountered: