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

[Big change][WIP] Server refactor #675

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

[Big change][WIP] Server refactor #675

wants to merge 12 commits into from

Conversation

JackUrb
Copy link
Contributor

@JackUrb JackUrb commented Sep 25, 2019

Description

The server.py file gradually grew from a bootstrapped tornado copy-paste into a full fledged server, but at no point had it ever been truly refactored. As such, the whole application was somewhat difficult to understand or debug, and making contributions to the server wasn't particularly clear.

This PR aims to refactor the server in a number of ways, with a few core changes that will be coming up in follow-on PRs that I will merge into this branch as they are accepted.

Explanation of changes

This specific PR starts the process by making 0 code logic changes. It simply splits up what used to be ~2000 lines of mostly undocumented code in one file into ~2000 lines of now documented code in separate files that describe their purposes.

Upcoming refactor steps

  • Split the monolithic server.py file into a number of more self-contained classes
  • Create a data_model that will abstract the operations that the server does on panes away from being directly in control of the handlers. Handlers should not be dereferencing arbitrary dicts. Move some of the server_utils into appropriate places following this.
  • Standardize the socket protocol between the forward and backwards sockets. Make an expected type for messages. Propagate this standardization to the javascript and python clients. Update the socket_handlers to use this new functionality.
  • Clean up the web_handlers.
  • Celebrate a nicer server, then move on to fixing __init__.py.

How Has This Been Tested?

Ran demo.py, all of the panes plot as expected. Will continue to test as such for each step. python -m visdom.server still functions as expected as well.

Types of changes

  • Better Engineering
  • Bug fixes

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • For JavaScript changes, I have re-generated the minified JavaScript code.

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

Successfully merging this pull request may close these issues.

2 participants