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

UI issue in the "tree view" if directory called "notebooks" is in the path. #7216

Open
avloss opened this issue Jan 21, 2024 · 7 comments
Open
Labels
Milestone

Comments

@avloss
Copy link

avloss commented Jan 21, 2024

Description

Getting a pesky warning in the "tree" view, if path to the directory includes "notebooks"(with that exact spelling).

Reproduce

  1. Create a path that includes "notebooks", and has sub-directories. ("http://localhost:8888/tree/test/notebooks/test")
  2. Navigate to that path via UI tree view.
  3. Click Refresh
  4. Get a pesky warning:
File Load Error for test
Invalid response: 404 Not Found
  1. Renaming directory "notebooks" to LITERALLY ANYTHING (including capitalising any of the letters) removes the issue.

Expected behavior

No warning happens
jupyter-tree-warning

@avloss avloss added bug status:Needs Triage Applied to issues that need triage labels Jan 21, 2024
@jtpio jtpio added this to the 7.0.x milestone Jan 22, 2024
@jtpio
Copy link
Member

jtpio commented Jan 22, 2024

Thanks @avloss for reporting the issue 👍

Just to double check, which version of Notebook 7 are you using?

@avloss
Copy link
Author

avloss commented Jan 24, 2024

Thanks @avloss for reporting the issue 👍

Just to double check, which version of Notebook 7 are you using?

notebook==7.0.7

@SATOSHIFNAKAMOTO
Copy link

Hello all 👋,

I've been able to reproduce the "tree view" issue on my end. After navigating to the /test/notebooks/test directory in the Jupyter Notebook UI, I clicked the browser's refresh button (specifically in Firefox, not the Jupyter in-app refresh), and I encountered the "File Load Error for test" stating "test is a directory, not a file".

Here's the detailed setup I'm working with:

Jupyter Notebook version: 7.1.0a2
Operating System: macOS 13.6.3 (22G436)
Browser: Firefox 121.0.1 (64-bit)
Attached is a screenshot showing the error message that appeared. It seems that the browser refresh is being misinterpreted, leading to this unexpected error. Has anyone had a similar experience, or does anyone have insights on what might be causing this behavior?

Best,
Satoshi 中本哲史

Screenshot 2024-01-24 at 10 09 00 AM

@itsmevichu
Copy link
Contributor

itsmevichu commented Feb 5, 2024

Hi @jtpio, I'm new to open source and very excited to contribute. I'd like to work on this issue. I have made a root cause analysis and here are the observations;

Background:

The current problem arises because the handlers are activated whenever specific keywords, namely {“notebooks”, “edits”, ”consoles”, ”terminals”}, are found in the path. This leads to undesired behaviour when directories with these keywords are present.
image

For instance:

Potential Solutions:

  1. Modify the regex of the handlers to activate only when the respective keyword is present at the beginning of the path.
    For example: NotebookHandler should only be activated when http://localhost:8888/**notebooks**/, not on http://localhost:8888/tree/**notebooks**/.
  2. Considering that these handlers are applied sequentially in the order they are appended, a quick workaround could involve appending the tree handler at the end (though not recommended).
    image

If you have any alternative ideas, please share them, and I'll explore them further to address the issue.

@jtpio
Copy link
Member

jtpio commented Feb 5, 2024

Thanks @itsmevichu for looking into this 👍

Yes it looks like there is an issue with how the handlers are set up. If you feel like opening a PR (even as a draft) to investigate these solutions, that would be great! That way it can be more easily tested, and maybe we can also look into adding a test to cover this case.

Thanks!

@itsmevichu
Copy link
Contributor

Sure @jtpio, I will create a PR and let you know.

@itsmevichu
Copy link
Contributor

@jtpio I have raised a PR #7253 please look into it.

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

No branches or pull requests

5 participants