Clean up datastore creator and get storeid from page config #7009
Conversation
|
Thanks for making a pull request to JupyterLab! To try out this branch on binder, follow this link: |
saulshanabrook
force-pushed
the
datastore-storeid
branch
from
05d0dcc
to
246fefd
Compare
| if self.store_id is not None: | ||
| # TODO: Temporary minimal reaction: | ||
| self.log.warning("Trying to reopen store that is already connected") | ||
| raise web.HTTPError(400) |
There was a problem hiding this comment.
The bit before this acts as a pretty decent placeholder (raises if trying to connect multiple times with the same store id).
There was a problem hiding this comment.
We should probably also fail here if the store_id is not given.
There was a problem hiding this comment.
I removed the backend knowledge of the store_id altogether. I actually think this is maybe ok? It just identifies users by connection. If you lose connection, then you make a new one and it treats you as a new peer.
There was a problem hiding this comment.
It possibly makes things like checkpoints a little harder. I'll make a separate issue for checkpoints. We can reintroduce this concept if needed after that.
There was a problem hiding this comment.
@saulshanabrook I'm happy for this to be in when you are. Once we merge our respective PRs we should be pretty close to being able to knit them together.
lock
bot
added
the
status:resolved-locked
Builds on #6871
TableManagerabstraction. We moved thechangedsignal to theTableManagerand will move the typing of the schema to the coreDatastoreobject in the future. Ideally, it would be nice to remove theTableManagerabstraction as well, eventually.Now this depends on jupyterlab/jupyterlab_server#74