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

SCT-401 - fix issue where losing connection results in user being unable to save #1571

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

briehl
Copy link
Member

@briehl briehl commented Jan 9, 2020

This scenario has been going on for years. A user logs in, does stuff, goes away for a while (or loses internet for a while), then returns to a disconnected kernel. After reconnecting or restarting the kernel, the user now can't save.

The reason for this was that there are two backend pieces that require their own state for KBase code to run. The kernel (where code gets executed, including app running and managing), and the server (where notebook/Narrative management gets done, including saving and loading).

Restarting the kernel does nothing to modify the server state. This change updates that so that both the kernel and server state get updated at the same time.

The real change is in kbaseNarrative.js. Now, when the kernel signals that its ready (on page load, or after a restart), 3 things happen:

  1. Push the current Narrative session info into the server.
  2. Push the current Narrative session info into the kernel.
  3. Forge the Jobs communication channel between the client and kernel, and have the kernel update its job state.

Previously, #2 and 3 were in separate responses to the same event, they're merged together to avoid a race condition (I'm not sure if that's ever been seen in the wild, but I think I've seen it once or twice if there's a very inconvenient network hiccup).

@briehl briehl requested a review from eapearson January 9, 2020 21:29
@briehl briehl changed the base branch from py3-pre-ee2 to develop January 16, 2020 18:53
@briehl
Copy link
Member Author

briehl commented Feb 4, 2020

Copying notes here from Slack, as effective to-dos for this PR:

  • Would be ideal if Narrative just tries to reconnect first, then falls back to restart, automatically via connection monitoring.
  • Avoid need for users to do this.
  • Should try to clear all "connection failed" messages in header when reconnected.
  • Make more clear what Kernel menu options do (maybe a help?)

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

Successfully merging this pull request may close these issues.

1 participant