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

Add current_active_notebook event #29

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Dexter2389
Copy link
Contributor

No description provided.

@Dexter2389 Dexter2389 requested a review from saggu June 18, 2024 19:14
@saggu
Copy link
Member

saggu commented Jun 18, 2024

I am testing this PR from the branch itself. The context id does not change between notebooks. Here are two instances of EventName NotebookName ContextID as I switch between them ,

CURRENT_ACTIVE_NOTEBOOK M18_evaluation_statistics.ipynb ContextSnapshot___59384bc7-48d1-43f6-9ac8-2f5316d5d1d7

CURRENT_ACTIVE_NOTEBOOK Untitled.ipynb ContextSnapshot___59384bc7-48d1-43f6-9ac8-2f5316d5d1d7

Note that the context id is the same. KNIC Engine is designed so that one context id is unique to a notebook name. I would prefer if the context id changes as well. @g1eb is that doable? Otherwise we will have to change engine so that a context can have multiple notebooks, which is a way bigger change.

@Dexter2389 you can take a look at this issue as well.

@saggu
Copy link
Member

saggu commented Jun 18, 2024

Had a discussion with @Dexter2389 . We have mongodb for session management, we should be able to utilize it to save the mapping between context id and notebook name. Here is the proposal ,

  1. When a notebook is opened, create a new context id and save in mongodb.
  2. Every request to knic-engine, will have a unique context id and notebook name as before.
  3. When user switches the notebook, CURRENT_ACTIVE_NOTEBOOK is generated, update the context id as well.
  4. When user clicks on END SESSION button in the companion, clear out context id - notebook name mapping in mongo db.
    @g1eb would this work?

@aphedges
Copy link
Contributor

aphedges commented Jun 19, 2024

@saggu, would this mapping be tracked in the companion back end or in the engine? It seems fine to use it as part of the existing companion back end session management, but adding it to the engine will make things much more complex.

I actually have a local branch to experiment with removing MongoDB completely because of the extra hassles is causes as part of deployment. Our session management library can work with other, more lightweight back ends than MongoDB. If you want me to show a PoC, I can clean up my branch and push it for you to see.

@saggu
Copy link
Member

saggu commented Jun 19, 2024

I was thinking about this, I think this mapping has to be managed in knic-jupyter, companion is not the right place. @Dexter2389 can you investigate this bit - storing this mapping in knic-jupyer.

@aphedges I am all for using light weight solutions, but ultimately @g1eb has to sign off on the change. Please do the required things, we'll see a demo when Gleb comes back.

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