-
Notifications
You must be signed in to change notification settings - Fork 316
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
Enable extensions to control the file_to_run #415
Conversation
Codecov Report
@@ Coverage Diff @@
## master #415 +/- ##
==========================================
+ Coverage 69.94% 70.09% +0.15%
==========================================
Files 57 57
Lines 6311 6340 +29
Branches 847 850 +3
==========================================
+ Hits 4414 4444 +30
+ Misses 1593 1591 -2
- Partials 304 305 +1
Continue to review full report at Codecov.
|
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, @afshin!
Great job @afshin. I am a bit late to the party... is the trait |
Maybe it could also be useful to document how to create a redirect handler using the Thinking of |
try: | ||
browser = webbrowser.get(self.browser or None) | ||
except webbrowser.Error as e: | ||
self.log.warning(_('No web browser found: %s.') % e) |
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.
Hi,
When I start jupyter from CUI, the program will stop with the following error.
...
File "/root/.pyenv/versions/3.7.3/lib/python3.7/site-packages/jupyter_server/serverapp.py", line 2020, in launch_browser
self.log.warning(_('No web browser found: %s.') % e)
UnboundLocalError: local variable '_' referenced before assignment
As shown in the error message, by removing the _
from the relevant part, jupyter can be executed successfully.
Looking at this script, it seems that underscores _
are inserted in self.log.info()
and so on.
I am not familiar with this notation, so I would like to confirm it.
Thanks in advance,
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.
Notebook server and Jupyter Server (due to its derivation from the former) define _
to be the method trans.gettext
such that any strings wrapped in _()
are candidates for translation lookups. However, this precludes the common use of _
to represent an ignored value in the same method - as is the case here (a few lines below):
assembled_url, _ = self._prepare_browser_open()
Seems like we should no longer return open_file
from _prepare_browser_open()
(since this is the only call to it) or adjust the name for the ignored value.
I don't know the history of using _
for trans.gettext
but using something like _i18n
instead seems more clear while not being too intrusive.
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 for the clarification!
Now I see how _
is used here, but as it is pointed out, it is neither clear nor safe usage, I guess...
This is a common way to represent ignored variables with _
in python, so I agree with @kevin-bates's suggestion!
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.
I've opened #424 to help avoid this in the future.
Enable extensions to control the file_to_run
In classic notebook, users could open a file directly, e.g.:
The server would start at the parent directory of
mynotebook.ipynb
and open the browser at/<base_url>/notebook/mynotebook.ipynb
. Alternatively, users could get the same behavior using the--NotebookApp.file_to_run
trait.Jupyter server still has the
file_to_run
trait, but the<base_url>/notebook
prefix is hard-coded, even though it doesn't make sense for server anymore. Further, extensions like JupyterLab cannot easily override this prefix.This PR adds a new configurable trait,
file_url_prefix
, to bothServerApp
andExtensionApp
. It changes the URL prefix where the server starts when a file_to_run is given. ExtensionApp can define their ownfile_url_prefix
default value to control this trait when they launch a server. This should fix jupyterlab/jupyterlab#8959 (comment).This PR also addresses some brittleness around corner cases where
file_to_run
androot_dir
are both given and verified by the new unit tests added.