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

add traitlet to ServePostProcessor that configures the browser used #618

Merged
merged 4 commits into from Jul 25, 2017

Conversation

Projects
None yet
3 participants
@mpacer
Copy link
Member

mpacer commented Jun 29, 2017

This closes #614.

I took some of the design of how the notebook implemented this and tried to abstract away only those parts that we needed.

In particular, I didn't run the browser.open command using threading.thread, since multi-threading didn't seem necessary, but that might be wrong. Could use others' insights.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Jul 3, 2017

I think that wrapping it in a thread is needed in some situations, because browser.open() might try to wait for the process it launches to exit before returning. Looking at the code of the webbrowser module, I think this would affect you if you set the BROWSER env variable and didn't already have the browser running when Python tried to launch it.

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jul 12, 2017

I believe it's only on Linux where open can block, and there only when the browser application is not already running, as Thomas mentioned.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Jul 13, 2017

I think it can happen on any system if you have the BROWSER environment variable set; that's registered as a GenericBrowser regardless of your platform. It's a corner case, but I suspect enough people will run into it to be worth considering - especially because the failure mode will be confusing: the browser will start and the server won't be running.

webbrowser.open(url, new=2)
try:
browser = webbrowser.get(self.browser or None)
threading.Thread(target=(browser.open(url, new=2))).start()

This comment has been minimized.

@takluyver

takluyver Jul 20, 2017

Member

This won't actually do it in a thread - you're calling browser.open() in the main thread, and passing its return value as the target= parameter.

You can either use a lambda, like we do in notebook, or pass args and kwargs separately, like this:

Thread(target=browser.open, args=(url,), kwargs={'new': 2})

Both ways are a bit ugly, unfortunately.

@takluyver takluyver added this to the 5.3 milestone Jul 25, 2017

@takluyver takluyver merged commit 4af6b64 into jupyter:master Jul 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mpacer mpacer added unlogged and removed unlogged labels Aug 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment