-
Notifications
You must be signed in to change notification settings - Fork 114
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
Added shutting down calls to the visualization server #241
Changes from 1 commit
f7474d9
a628c1c
1c09a99
a49c584
52a0bca
8b48732
d2d4940
e47906a
4dd240a
f77b84a
5d99580
63756c3
d2e91cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Fixes #232
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,105 @@ | ||
#!/usr/bin/env python | ||
|
||
from __future__ import print_function | ||
|
||
import os | ||
import sys | ||
import signal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to stay in for Python 2/3 compat. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I totally missed that. Thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe make a comment in your PR that says why that is there. |
||
import socket | ||
import webbrowser | ||
if sys.version_info < (3, 0): | ||
from SimpleHTTPServer import SimpleHTTPRequestHandler | ||
from BaseHTTPServer import HTTPServer | ||
else: | ||
from http.server import SimpleHTTPRequestHandler | ||
from http.server import HTTPServer | ||
|
||
|
||
__all__ = ['run_server'] | ||
|
||
|
||
def run_server(port=8000,scene_file="Null"): | ||
#change dir to static first. | ||
os.chdir("static/") | ||
HandlerClass = SimpleHTTPRequestHandler | ||
ServerClass = HTTPServer | ||
Protocol = "HTTP/1.0" | ||
server_address = ('127.0.0.1', port) | ||
HandlerClass.protocol_version = Protocol | ||
httpd = ServerClass(server_address, HandlerClass) | ||
sa = httpd.socket.getsockname() | ||
print("Serving HTTP on", sa[0], "port", sa[1], "...") | ||
print("hit ctrl+c to stop the server..") | ||
print("To view visualization, open:\n") | ||
url = "http://localhost:"+ str(sa[1]) + "/index.html?load=" + scene_file | ||
print(url) | ||
webbrowser.open(url) | ||
httpd.serve_forever() | ||
|
||
|
||
if __name__ == "__main__": | ||
run_server() | ||
import BaseHTTPServer | ||
from SimpleHTTPServer import SimpleHTTPRequestHandler | ||
|
||
|
||
__all__ = ['Server'] | ||
|
||
|
||
class StoppableHTTPServer(BaseHTTPServer.HTTPServer): | ||
""" | ||
Overrides BaseHTTPServer.HTTPServer to include a stop | ||
function. | ||
""" | ||
|
||
def server_bind(self): | ||
BaseHTTPServer.HTTPServer.server_bind(self) | ||
self.socket.settimeout(1) | ||
self.run = True | ||
|
||
def get_request(self): | ||
while self.run: | ||
try: | ||
sock, addr = self.socket.accept() | ||
sock.settimeout(None) | ||
return (sock, addr) | ||
except socket.timeout: | ||
pass | ||
|
||
def stop(self): | ||
self.run = False | ||
|
||
def serve(self): | ||
while self.run: | ||
try: | ||
self.handle_request() | ||
except TypeError: | ||
# When server is being closed, while loop can run once | ||
# after setting self.run = False depending on how it | ||
# is scheduled. | ||
pass | ||
|
||
|
||
class Server(object): | ||
""" | ||
Parameters | ||
---------- | ||
:param port : integer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We try to follow the numpydoc style for docstrings, so this would be:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
Defines the port on which the server will run. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't the case if the port is taken. |
||
:param scene_file : name of the scene_file generated for visualization | ||
Used here to display the url | ||
:param directory : path of the directory which contains static and scene files. | ||
Server is started in this directory itself. | ||
|
||
Example | ||
------- | ||
>>> server = Server(scene_file=_scene_json_file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't indent here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
>>> server.run_server() | ||
|
||
""" | ||
def __init__(self, port=8000, scene_file="Null"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we need to concatenate it with a string and None will raise an error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, user must know that the there is no file name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And I don't know it yet but the url we get from "Null" can be used to give user an option to choose the json files like in ipython notebook. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can worry about that when the feature is implemented. I think scene_file should be a required arg. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I change it to None? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or make it a required argument? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to me like it is a requirement. What use is the server if I don't have a scene file? I guess you could load it from a url |
||
self.port = port | ||
self.scene_file = scene_file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My suggestion in the previous commit was to have this call sig: def __init__(scene_file, directory=None, port=8000) The scene file should be required and the directory can default to the cwd if None is passed in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add that in my new pr on static directory creation. Trying to make it atomic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this is a whole new server code. It seems atomic to add this. |
||
self.directory = "static/" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this should be pass in as an argument. |
||
|
||
def run_server(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if port 8000 is taken? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It throws an error. This needs to be fixed. |
||
# Change dir to static first. | ||
os.chdir(self.directory) | ||
handler_class = SimpleHTTPRequestHandler | ||
server_class = StoppableHTTPServer | ||
protocol = "HTTP/1.0" | ||
server_address = ('127.0.0.1', self.port) | ||
handler_class.protocol_version = protocol | ||
self.httpd = server_class(server_address, handler_class) | ||
sa = self.httpd.socket.getsockname() | ||
print("Serving HTTP on", sa[0], "port", sa[1], "...") | ||
print("To view visualization, open:\n") | ||
url = "http://localhost:"+str(sa[1]) + "/index.html?load=" + \ | ||
self.scene_file | ||
print(url) | ||
webbrowser.open(url) | ||
print("Hit Ctrl+C to stop the server...") | ||
signal.signal(signal.SIGINT, self.stop_server) | ||
self.httpd.serve() | ||
|
||
def stop_server(self, signal, frame): | ||
""" | ||
Confirms and stops the visulisation server | ||
:param signal: | ||
Required by signal.signal | ||
:param frame: | ||
Required by signal.signal | ||
|
||
""" | ||
res = raw_input("Shutdown this visualization server ([y]/n)? ") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fail in Python 3. Oliver had the fix for this in this file before this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are the options then, should I use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it! |
||
if res is (None or 'y'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this eval to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, I must add it as well |
||
print("Shutdown confirmed") | ||
print("Shutting down server...") | ||
self.httpd.stop() |
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.
We shouldn't do this. Just make sure to use parentheses on all print statements.
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.
Okay