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

Temp notebook copies and kernels never removed #51

Closed
spokeydokeys opened this issue May 6, 2020 · 17 comments
Closed

Temp notebook copies and kernels never removed #51

spokeydokeys opened this issue May 6, 2020 · 17 comments

Comments

@spokeydokeys
Copy link

When I launch an apps link, the my notebook is correctly copied and a new kernel started; however, they are never deleted/shutdown, which leaves me with a large number of junk files and running kernels. Is there a config I should modify to ensure this happens?

Win 10 Pro 64bit
python 3.7.5
appmode 0.7.0
jupyter-core 4.6.1
jupyter-client 5.3.4

@oschuett
Copy link
Owner

oschuett commented May 8, 2020

The cleanup is triggered via window.onbeforeunload. Unfortunately, this is not very reliable because browsers are hesitant to execute javascript after the user left the page.

Do you see the DELETE requests in the server logs?

@spokeydokeys
Copy link
Author

I don't see any delete commands. Here are the lines for opening and closing of an appmode notebook in Chrome Version 81.0.4044.129:

[I 10:07:08.331 NotebookApp] Appmode get: AppUi.ipynb
[I 10:07:08.332 NotebookApp] Appmode creating tmp copy: /.AppUi-5.ipynb
[I 10:07:09.465 NotebookApp] Kernel started: 82b4101e-5bf4-4a47-9a3b-7239d338d035
<<<< Note: Appmode Notebook closed here >>>>
[I 10:07:22.257 NotebookApp] Starting buffering for 82b4101e-5bf4-4a47-9a3b-7239d338d035:f24cc1af975b43e98d1baee777de24ff

@oschuett
Copy link
Owner

oschuett commented May 17, 2020

It indeed seems like the DELETE requests are not send. Are there any messages in the browser's web console? As said before, window.onbeforeunload is unfortunately known to be unreliable.

@oschuett oschuett added the FAQ label May 17, 2020
@yakutovicha
Copy link

We have also noticed this problem. One little detail: the apps do get closed in Firefox, but not in Chrome.

@oschuett
Copy link
Owner

oschuett commented Jun 1, 2020

Apparently, there was a recent change in chrome, which disallow sync XHR in page dismissal.

Could you once try if setting async: true in main.js:25 helps?

@yakutovicha
Copy link

Could you once try if setting async: true in main.js:25 helps?

Yes, I tried: the problem gets resolved for Chrome, but shows up for Firefox.

@oschuett
Copy link
Owner

oschuett commented Jun 2, 2020

Ok, it seems the proper way these days is sendBeacon(). That API only sends POST requests. So, the server side has to be adopted as well.

@yakutovicha, do you want to give it a try? I'm not sure I'll find time for it this week.

@ltalirz
Copy link

ltalirz commented Jun 2, 2020

Just a shot in the dark but what about using the more generic fetch API in order to be able to keep using a DELETE request and not modify the backend?
Browser compatibility seems to be the same.

See here for an example of replacing a sendBeacon() with a fetch call.

@oschuett If you could have a guess as to how the fetch call should look like, we could test it out & iterate.

@oschuett
Copy link
Owner

oschuett commented Jun 2, 2020

I think, sendBeacon() is the safer choice because it's explicitly designed to work with onbeforeunload. Overall the changes - also on the server side - should be rather straight forward.

@yakutovicha
Copy link

I think, sendBeacon() is the safer choice because it's explicitly designed to work with onbeforeunload. Overall the changes - also on the server side - should be rather straight forward.

Hi @oschuett. I am personally not very experienced in javascript, so I would really leave the fix to somebody who knows better what they are doing. However, I was wondering, if it would make sense to do a temporary fix of a sort: if chrome then async: true, else async: false?

I can do that rather quickly. And then we could wait for a more future-proof fix.

Let me know.

@oschuett
Copy link
Owner

oschuett commented Jun 6, 2020

I took a first stab at it. Let me know if it works for you.

@yakutovicha
Copy link

thanks, @oschuett, it seems to work! I checked on Firefox, Chrome, Safari

@EngineerReversed
Copy link

EngineerReversed commented Jun 8, 2020

thanks, @oschuett, it seems to work! I checked on Firefox, Chrome, Safari

I tried the above changes but it did not work for me. On looking into jupyter logs, I found the following :

405 POST /apps/notebooks/notebooks/.example.ipynb (172.17.0.5) 2.05ms referer=<link>

Is it due to the fact that it is not able to delete the file as it doesn't have permissions?

Chrome version : Version 80.0.3987.163 (Official Build) (64-bit)

PS : I am using docker with appmode installation in root mode and accesing jupyter notebook in user mode.

@oschuett
Copy link
Owner

oschuett commented Jun 8, 2020

@EngineerReversed, it's hard to say without more information.
Maybe you can run with jupyter-notebook --log-level=DEBUG ?

@EngineerReversed
Copy link

EngineerReversed commented Jun 8, 2020

@EngineerReversed, it's hard to say without more information.
Maybe you can run with jupyter-notebook --log-level=DEBUG ?

I tried the same on my local mac and it was able to delete temporary copies of notebook.

[I 17:47:26.010 NotebookApp] Appmode get: Untitled.ipynb
[I 17:47:26.012 NotebookApp] Appmode creating tmp copy: /.Untitled-0.ipynb
[I 17:47:27.917 NotebookApp] Kernel started: 27c5fadb-eaac-4085-9ffe-eefdbadd039d
[I 17:47:30.229 NotebookApp] Adapting to protocol v5.1 for kernel 27c5fadb-eaac-4085-9ffe-eefdbadd039d
[I 17:47:46.272 NotebookApp] Starting buffering for 27c5fadb-eaac-4085-9ffe-eefdbadd039d:688c71af33ae400489a16697320c2ff4
[I 17:47:46.305 NotebookApp] Appmode deleting: .Untitled-0.ipynb

I guess it has something to do with permissions on server.

@EngineerReversed
Copy link

On further series of experiments, I was able to localize the bug. I had not uninstalled the previous version of appmode and install newer version after cloning from github using python setup.py install.
It also did not allow nbextention to be enabled as well.

Enabling notebook extension appmode/main...
      - Validating: problems found:
        - require?  X appmode/main

After installing it via pip install . and enabling appmode in extensions, it deleted the temporary copies successfully.

[I 19:37:00.594 NotebookApp] Appmode get: notebooks/notebooks/Taxonomy_EDA_Demo.ipynb
[I 19:37:00.594 NotebookApp] Appmode creating tmp copy: notebooks/notebooks/.Taxonomy_EDA_Demo-0.ipynb
[I 19:37:03.353 NotebookApp] Kernel started: 67a90f1b-e48f-48ce-9ecb-8db328c06228
[I 19:37:04.076 NotebookApp] Adapting to protocol v5.1 for kernel 67a90f1b-e48f-48ce-9ecb-8db328c06228
[I 19:37:34.060 NotebookApp] Appmode deleting: notebooks/notebooks/.Taxonomy_EDA_Demo-0.ipynb
[I 19:37:34.363 NotebookApp] Kernel shutdown: 67a90f1b-e48f-48ce-9ecb-8db328c06228
[W 19:37:34.364 NotebookApp] Skipping trash for /home/jupyter/notebooks/notebooks/.Taxonomy_EDA_Demo-0.ipynb, on different device to home directory
[W 19:37:34.368 NotebookApp] zmq message arrived on closed channel

I took a first stab at it. Let me know if it works for you.

You can release a new version with this patch.

@oschuett
Copy link
Owner

oschuett commented Jun 9, 2020

Alright, I've made a new release. Thanks for your 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

No branches or pull requests

5 participants