-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
Ignore pageconfig file if JSON is zero-length #444
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @holzman
Can pageconfig include security related options? I think yes. This I think it's better to crash than to allow the server to start with the options inactive. Of course that should be with a better error message |
Regardless of if it's a security issue, from a user perspective it can be very annoying/surprising to have a config file (effectively) silently ignored due to a syntax error. There may be log messages, but people generally don't look at them unless there's a problem, which may not be immediately apparent since JupyterLab will start. Same for jupyter-server/jupyter_server#1404 In the motivating issue jupyter-server/jupyter_server#1403 the problem is a lack of disk space, which is going to cause many more problems than an invalid empty config file. |
In our particular case, the disk being full was transient, but the zero-length page_config.json persisted. How about if we check if the file exists and isn't zero bytes? That sounds like it will cover all of the bases - ignoring an empty file seems OK to me. |
Sounds reasonable to me! |
@krassowski does the final version is ok for you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #443