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

Don't die if template path cannot be read #2162

Merged

Conversation

stuaxo
Copy link
Contributor

@stuaxo stuaxo commented Jul 6, 2024

Ordinary users may not have read access to conf in system directories,

In those cases .exists() will trigger a PermissionError when enumerating the conf directories.

Ordinary users may not have read access to conf in system directories,

In those cases .exists() will trigger a PermissionError
@stuaxo
Copy link
Contributor Author

stuaxo commented Jul 6, 2024

This may be jupyter/jupyter#634

@takluyver takluyver added the bug label Jul 31, 2024
@takluyver
Copy link
Member

Is this sufficient to avoid the problem you encountered? It looks like there are other exists() checks in get_template_names() which could also run into permission errors.

(It also seems weird to me that standard system directories under /usr wouldn't be readable, but I agree Jupyter should ideally cope with that, even if it's unusual)

@stuaxo
Copy link
Contributor Author

stuaxo commented Jul 31, 2024

It was sufficient for the bug I was seeing, it's certainly possible there could be other places with the same issue.

@stuaxo
Copy link
Contributor Author

stuaxo commented Jul 31, 2024

(I also found the fact .exists() can throw a permission error suprising, but it seems to be a thing.

@takluyver
Copy link
Member

Fair enough. I'm just going to close & reopen this to re-run the tests with the fix from #2168.

It certainly can be surprising, but I think I prefer it to pretending inaccessible folders are empty; glob does behave like that, and I've certainly been caught by it before.

@takluyver takluyver closed this Aug 7, 2024
@takluyver takluyver reopened this Aug 7, 2024
@takluyver takluyver merged commit 55ff3e9 into jupyter:main Aug 7, 2024
40 of 44 checks passed
@stuaxo
Copy link
Contributor Author

stuaxo commented Aug 8, 2024

Cheers, I think this will help others who do what I did and combine homebrew and virtualenvs.

@stuaxo stuaxo deleted the conf-reading-dont-die-on-permission-error branch August 8, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants