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

Feature: Terminal support websocket reconnection #4802

Merged
merged 2 commits into from Jun 29, 2018

Conversation

@afshin
Copy link
Member

@afshin afshin commented Jun 28, 2018

NOTE: I accidentally closed @perrywky's original PR because I tried to push a commit to it then to amend that commit but since his PR was from his master branch I got confused and botched it.

Fixes #4763

@perrywky's original description:

  1. the solution is very like kernel's reconnection mechanism, listening to the close event, and try reconnecting until reach max attemption limit
  2. users can refresh terminal by themselfs, but it will clean the terminal after reconnection, because after reconnection, terminado will sent a few latest messages of current session, which is duplicated
  3. in this auto reconnection scenario, i'll ignore these duplicated messages, and wait for the 'setup' message sent by terminado, and treat it as the finish of a successful reconnection, in this way, user will not sense anything, except a few lags, which i think is more user friendly.
@afshin afshin force-pushed the perrywky-master branch from 3249b0e to 7632139 Jun 28, 2018
@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jun 29, 2018

Thanks @perrywky!

@ian-r-rose ian-r-rose merged commit 90d865b into jupyterlab:master Jun 29, 2018
2 checks passed
@afshin afshin deleted the perrywky-master branch Jul 5, 2018
@afshin afshin mentioned this pull request Jul 5, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants