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

Notebook server info files #4772

Merged
merged 5 commits into from Jan 14, 2014
Merged

Notebook server info files #4772

merged 5 commits into from Jan 14, 2014

Conversation

takluyver
Copy link
Member

This adds info files for running notebook servers, akin to the connection files we already write for kernels, along with a function to discover them. This should facilitate making tools that can open notebooks without starting a new server for each one.

What values are serialised and what names they're given are up for discussion. I've currently put in the URL and its component parts and the root directory where the server is running.

self.remove_server_info_file()


def discover_running_servers(profile='default'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this should be defined somewhere other than the notebookapp file, but I'm not sure where. Any idea?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a brief look, but this seemed the most logical place I could see. I'd argue that it's effectively 'list instances of NotebookApp on this system', so it makes sense to define it near the NotebookApp class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to add an ipython notebook list subcommand to show currently running servers?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am +1 on having a notebook list subcommand, but I think we should name the function using "list" instead of "discover": discover_running_servers.

@ccordoba12
Copy link
Member

In case the server fails to start, could the file contain the corresponding traceback error? That would be useful to show possible errors to users in a GUI dialog when the server is started in the background.

@takluyver
Copy link
Member Author

The idea is that the file only exists when the server is running, so if it fails to start, the file shouldn't be present.

Perhaps there should be an option which makes the server emit more structured (e.g. JSON) messages on stdout, for a controlling process to monitor. Another example would be for synchronisation - you don't want to try to read the server info file until the server has finished writing it.

'secure': bool(self.certfile),
'baseurlpath': self.base_project_url,
'notebookdir': os.path.abspath(self.notebook_manager.notebook_dir),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably keep the same names in JSON, so base_project_url and notebook_dir.

@ellisonbg
Copy link
Member

Overall, I think this looks good, just some minor changes to make.

@takluyver
Copy link
Member Author

  • Added ipython notebook list subcommand - I think there's some work to be done on simplifying defining subcommands, but that's for another PR.
  • Function is list_running_servers
  • Renamed 2 keys as Brian suggested.

if self.json:
print(json.dumps(serverinfo))
else:
print(serverinfo['url'], "::", serverinfo['notebook_dir'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to add [...] so that the full JSON output is valid JSON, not just each entry?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in dev meeting. Conclusion: it's more trouble than it's worth to get commas in the right places, so I'm leaving it as one JSON object per line for now.

minrk added a commit that referenced this pull request Jan 14, 2014
@minrk minrk merged commit 033b948 into ipython:master Jan 14, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@takluyver takluyver deleted the nbserver-files branch July 28, 2017 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants