Open stela2502 opened 4 years ago
Hi @MSeal. I seam to not have been specific here: The situation this bug occurs is following:
(1) root user installs nbconvert globaly in a singularity image (2) a different user than the one creating the singularity image uses the image (3) the image's purpose is to run a jupyter notebook with access ro Python and R kernerl's (not sure if that is important)
The command to start jupyter is this: jupyter lab --port 9734 --ip=0.0.0.0 --allow-root --no-browser
I understand that this is a quite specific setting, but the fact that you already had an issue here describing the problems in a very detailed way shows that this problem is not neglectable. The fact that you nevertheless neglect it is surprising to me.
In your answer you state that my fix to the uncatched error state of your package is endangering other parts of your package? You could easily throw another error/warning in the at the moment unused except block.
I am sorry to have called the code immature. I am just shocked that a quite important part of a quite important software is failing due to access right problems in a one year old bug report. And that this 'issue' was actually closed including a path to solve the problem. But still the issue was not solved. Especially the fact that rendering the package useless in certain situations is better than to not do what can not be done (create a path where you do not have the access rights) and proceed with another path where you ultimately can write the file o write and retain the usability of the tool.
Please expand on the supposed side effects of this simple fix and compare the severity of these side effects to the described bug. Thank you!
And here a deep link to the issue that describes the bug: https://github.com/jupyter/nbconvert/issues/1167
So a couple things to note. First is that issues are sometimes open for a while if there's not a clear resolution or available contributor to fix. This project is supported by volunteers and Covid has not been additive to the maintainer count this year. E.g. I'm going to be offline for a few weeks starting later today. In this case I think the resolution is not clear or the problem is outside the scope of the codebase.
The reason for this is that if a user installs into root directories that don't have read permissions for nbconvert and then it's really more of a setup problem than a project problem. Have you identified the folders that are not accessible and why there's no read permission to then first?
The issue with silently skipping config loads is that some templates are modified by config defaults and not having those loadable means unexpected changes in behavior. I'm maybe ok with a very clear warning if a particular config file can't be read but not comfortable with just skipping all config load on a particular file load issue.
Does that make sense? Maybe @maartenbreddels or @SylvainCorlay can help with this issue moving forward, as I won't be available to respond for a long while.
@stela2502 regarding the technical issue:
The way Jupyter works with respect to configuration and data directories, is that it first goes through
system
directoriesprefix
directoriesuser
filesand overrides the system configuration with the prefix configuration, and the prefix configuration with the user configuration. So system and prefix directories must be readable for the user. It is most typically true, and I have never encountered a case where it was not so far. (The classic notebook, lab, hub, etc...) If there is a bug in nbconvert (such as opening a file with write mode while it is not needed), please let use know.
The fact that you nevertheless neglect it is surprising to me.
I am sorry to have called the code immature. I am just shocked that a quite important part of a quite important software is failing due to access right problems in a one year old bug report. And that this 'issue' was actually closed including a path to solve the problem. But still the issue was not solved.
The entire Jupyter stack, used by millions of users and hundreds of corporations is maintained by a handful of developers, who deal with technical debt, implementing new features, handling bug reports, packaging the software on all platforms, organizing community events, and more! We do not ignore people on purpose, but it is only natural if all issues don't get answers and fall through the cracks. I am a "maintainer" on hundreds of repositories, and keeping track of all GitHub notifications (yet alone GitHub mentions) is simply impossible.
The issue with silently skipping config loads ...
Back to the technical issue. I agree with @MSeal that we need to be able to read the data directory of the prefix where the package is installed at least.
Hi, I encountered the same issue as you, @stela2502 . First of all, @MSeal and @SylvainCorlay thank you for the clarifications you provided, and thank you for the work you do with maintaining open-source software in general. Similarly to @stela2502 , I'd like to see this issue resolved, because it disrupts the way I used to configure and setup my Notebooks so far. I’d like to come up with a solution, which would be acceptable by all parties. Hence, I've been trying to determine what has actually changed since I set up my last Jupyter Notebook instance (in August 2019). Below you will find the analysis I performed and the conclusions I came up with.
In August 2019 I used Debian 9.12:
lsb_release -a
No LSB modules are available. Distributor ID: Debian Description: Debian GNU/Linux 9.12 (stretch) Release: 9.12 Codename: stretch
with the following Jupyter versions:
$ jupyter --version
jupyter core : 4.6.3 jupyter-notebook : 6.1.4 qtconsole : not installed ipython : 7.9.0 ipykernel : 5.3.4 jupyter client : 6.1.7 jupyter lab : not installed nbconvert : 5.6.1 ipywidgets : 7.5.1 nbformat : 5.0.8 traitlets : 4.3.3
Now (January 2021), I've been trying to set up a Jupyter Notebook the same way with Ubuntu 20.04.01
lsb_release -a
No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.1 LTS Release: 20.04 Codename: focal
with the following Jupyter versions:
jupyter --version
jupyter core : 4.7.0 jupyter-notebook : 6.1.6 qtconsole : 5.0.1 ipython : 7.19.0 ipykernel : 5.4.2 jupyter client : 6.1.7 jupyter lab : not installed nbconvert : 6.0.7 ipywidgets : 7.6.2 nbformat : 5.0.8 traitlets : 5.0.5
Actually, I'm running Jupyter Notebook in a virtual Python environment, but it shouldn't be the case because I did so in both in August 2019, as well as now.
In both cases Jupyter paths are virtually the same:
jupyter --path
config: /home/MY-USER/.jupyter /home/MY-USER/.local/share/virtualenvs/MY-USER_VENV-ID/etc/jupyter /usr/local/etc/jupyter /etc/jupyter
Also, for both systems, there are no /usr/local/etc/jupyter
nor /etc/jupyter
directories (system
directories).
As it seems that the problem concerns nbconvert
, I’ll focus on what changed in nbconvert/exporters/templateexporter.py
between August 2019 and January 2021.
It seems that the problem is with the function _get_conf(self):
. It was introduced in the following commit: https://github.com/jupyter/nbconvert/commit/2e3bc992caf5f21a491567922dc620ef21f90540#diff-d7f1d14336754d94d19e61c0696fa7fbebd712447a91a91fafff98fe9130713b
So, that function utilizes a mechanism from pathlib
library to find out wether configuration path exists (https://docs.python.org/3/library/pathlib.html):
if conf_path.exists():
(https://github.com/jupyter/nbconvert/commit/2e3bc992caf5f21a491567922dc620ef21f90540#diff-d7f1d14336754d94d19e61c0696fa7fbebd712447a91a91fafff98fe9130713bR482)
Unfortunately, this rises the following error:
… if conf_path.exists(): File "/usr/lib/python3.8/pathlib.py", line 1388, in exists self.stat() File "/usr/lib/python3.8/pathlib.py", line 1194, in stat return self._accessor.stat(self) PermissionError: [Errno 13] Permission denied: '/usr/share/jupyter/nbconvert/templates/conf.json'
Why? Because pathlib
is unable to determine whether this directory or file really doesn’t exist or just the user doesn’t have permission to read from this directory. @stela2502 ’s solution was to ignore this error, as it is in fact just the permission error. I agree with @MSeal that it should throw “a very clear warning” instead of just ignoring it whatsoever — in case people would like to use system-wide templates (assuming that /usr/share/jupyter/nbconvert/templates/conf.json
really exists) and just had permission issues. On the other hand, if you are dealing with system-wide templates available for all users, you better know what you are doing and simply not-seeing system-wide defined templates used in your Notebook should ring some bells ("hmm, maybe it is due to permissions"). Meanwhile I guess that it is more common to just have predefined paths, and simply use Jupyter as-it-is, without defining some new templates. Then, making as default the mechanism of getting configuration from system-wide directories (by default unavailable for normal users) is in my opinion just wrong from the point of view of usability. Again, I don't even try to use Jupyter Notebook outside of the virtual environment. Maybe it is not an issue when notebook
is installed system-wide, e.g., with pip
. I’d have to check it, but anyway I’d like to have this working for virtual environment Notebooks as well.
To sum up, @MSeal and @SylvainCorlay, in the light of the above analysis, are you willing to accept the solution provided by @stela2502 with an additional “very clear warning”? I can create a pull request for that, but first, I’d like to know that we all agree on such solution.
EDIT: Provided a link to stela's solution.
I'd be ok with a warning only after passing an argument to nbconvert that says it can ignore config paths. The problem with just a warning is that if it doesn't stop the user they will usually ignore it and report more esoteric issues later as a result. And thanks for offering to make a patch!
To be clear I only use Notebooks from virtual environments and never have issues. It's a permissions to /usr/share/jupyter/nbconvert
that's the root issue here. That's been a stable path used by nbconvert for many years to access files. What changed in 6.0 is it's eagerly checking this path for config files to see if any system wide settings are in place. Not having read permissions to /usr/share
paths is an oddity for a unix-like file system as many systems assume they can access it (e.g. for IBM's description).
I am not following what this means, since to reach said code one must have installed nbconvert... since the code being changed is inside nbconvert. I'm going to close this PR for now, as I don't think the change in behavior as written in the PR is something we want added. If you'd like to open an issue that explains the problem you ran into in more depth that would be preferred.
Also please refrain from name calling as well. We have a code of conduct if you're not aware, and calling code you're unfamiliar with immature could be interpreted as insulting to some maintainers or contributors. I'm assuming you meant no insult, but I'd like to point out that it's ambiguous to me here what the comment's intent was.
Originally posted by @MSeal in https://github.com/jupyter/nbconvert/pull/1450#issuecomment-709650862