-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Reloader option. Some ideas #1094
Comments
Is there a tangible problem that needs to be solved? I do not see any noticeable delay from watching all modules, instead of just the user provided. It's unnecessary work, but as long as it runs fast enough and does not impact the application performance, I'd prefer a simple and short implementation over a more sophisticated one. |
I will try to discover what is the problem and I will report with more details. |
Hi!
https://docs.python.org/2/library/thread.html What would it happen if the code of some server catch the exception unintentionally? The thread checking changes in files raise and exception in the main thread and exits. The main thread catch the exception, maybe logging that some exception happened but it continues its executions (and the process doesn't exit). Now the web server continue serving pages but there is no thread checking changes in files because that thread exited before, and no new process was created. For some reason, maybe this is happening to me:
I analyzed the reloader code of werzkzeug: In that code the server is running in a thread and the code checking for changes in python modules and exiting the application for a new reload is in the main thread. It's the opposite of the current implementation in bottle. I made this changes in bottle and it works for me. I have no problem: def run(app=None,
server='wsgiref',
host='127.0.0.1',
port=8080,
interval=1,
reloader=False,
quiet=False,
plugins=None,
debug=None,
config=None, **kargs):
""" Start a server instance. This method blocks until the server terminates.
:param app: WSGI application or target string supported by
:func:`load_app`. (default: :func:`default_app`)
:param server: Server adapter to use. See :data:`server_names` keys
for valid names or pass a :class:`ServerAdapter` subclass.
(default: `wsgiref`)
:param host: Server address to bind to. Pass ``0.0.0.0`` to listens on
all interfaces including the external one. (default: 127.0.0.1)
:param port: Server port to bind to. Values below 1024 require root
privileges. (default: 8080)
:param reloader: Start auto-reloading server? (default: False)
:param interval: Auto-reloader interval in seconds (default: 1)
:param quiet: Suppress output to stdout and stderr? (default: False)
:param options: Options passed to the server adapter.
"""
if NORUN: return
if reloader and not os.environ.get('BOTTLE_CHILD'):
import subprocess
lockfile = None
try:
fd, lockfile = tempfile.mkstemp(prefix='bottle.', suffix='.lock')
os.close(fd) # We only need this file to exist. We never write to it
while os.path.exists(lockfile):
args = [sys.executable] + sys.argv
environ = os.environ.copy()
environ['BOTTLE_CHILD'] = 'true'
environ['BOTTLE_LOCKFILE'] = lockfile
p = subprocess.Popen(args, env=environ)
while p.poll() is None: # Busy wait...
os.utime(lockfile, None) # I am alive!
time.sleep(interval)
if p.poll() != 3:
if os.path.exists(lockfile): os.unlink(lockfile)
sys.exit(p.poll())
except KeyboardInterrupt:
pass
finally:
if os.path.exists(lockfile):
os.unlink(lockfile)
return
try:
if debug is not None: _debug(debug)
app = app or default_app()
if isinstance(app, basestring):
app = load_app(app)
if not callable(app):
raise ValueError("Application is not callable: %r" % app)
for plugin in plugins or []:
if isinstance(plugin, basestring):
plugin = load(plugin)
app.install(plugin)
if config:
app.config.update(config)
if server in server_names:
server = server_names.get(server)
if isinstance(server, basestring):
server = load(server)
if isinstance(server, type):
server = server(host=host, port=port, **kargs)
if not isinstance(server, ServerAdapter):
raise ValueError("Unknown or unsupported server: %r" % server)
server.quiet = server.quiet or quiet
if not server.quiet:
_stderr("Bottle v%s server starting up (using %s)...\n" %
(__version__, repr(server)))
_stderr("Listening on http://%s:%d/\n" %
(server.host, server.port))
_stderr("Hit Ctrl-C to quit.\n\n")
if reloader:
import signal
signal.signal(signal.SIGTERM, lambda *args: sys.exit(0))
t = threading.Thread(target=server.run, args=(app,))
t.setDaemon(True)
t.start()
lockfile = os.environ.get('BOTTLE_LOCKFILE')
exists = os.path.exists
mtime = lambda p: os.stat(p).st_mtime
files = dict()
for module in list(sys.modules.values()):
path = getattr(module, '__file__', '')
if path[-4:] in ('.pyo', '.pyc'): path = path[:-1]
if path and exists(path): files[path] = mtime(path)
while True:
if not exists(lockfile)\
or mtime(lockfile) < time.time() - interval - 5:
sys.exit(3)
for path, lmtime in list(files.items()):
if not exists(path) or mtime(path) > lmtime:
sys.exit(3)
time.sleep(interval)
else:
server.run(app)
except KeyboardInterrupt:
pass
except (SystemExit, MemoryError):
raise
except:
if not reloader: raise
if not getattr(server, 'quiet', quiet):
print_exc()
time.sleep(interval)
sys.exit(3) I deleted t = threading.Thread(target=server.run, args=(app,))
t.setDaemon(True)
t.start() |
I tried to research the origin of that prints in the terminal. I did this in
The print of that exception is done here:
This is the code in def handle_error(self, request, client_address):
"""Handle an error gracefully. May be overridden.
The default is to print a traceback and continue.
"""
print '-'*40
print 'Exception happened during processing of request from',
print client_address
import traceback
traceback.print_exc() # XXX But this goes to stderr!
print '-'*40 This is all the inheritance chain:
WSGIServer inherits from HTTPServer class WSGIServer(HTTPServer): as you can see in
class HTTPServer(SocketServer.TCPServer): as you can see here
And finally I was not wrong with my intuition. def _handle_request_noblock(self):
"""Handle one request, without blocking.
I assume that select.select has returned that the socket is
readable before this function was called, so there should be
no risk of blocking in get_request().
"""
try:
request, client_address = self.get_request()
except socket.error:
return
if self.verify_request(request, client_address):
try:
self.process_request(request, client_address)
except:
self.handle_error(request, client_address)
self.shutdown_request(request)
else:
self.shutdown_request(request) In that place, the |
No server software should catch and ignore |
Hmm, it catches Starting the server in a thread different than main has other problems, unfortunately. For example, killing it by setting it to deamon and exiting the main-thread would ignore any exception handlers or cleanup code in the application itself :/ |
def _handle_request_noblock(self):
"""Handle one request, without blocking.
I assume that selector.select() has returned that the socket is
readable before this function was called, so there should be no risk of
blocking in get_request().
"""
try:
request, client_address = self.get_request()
except OSError:
return
if self.verify_request(request, client_address):
try:
self.process_request(request, client_address)
except Exception:
self.handle_error(request, client_address)
self.shutdown_request(request)
except:
self.shutdown_request(request)
raise
else:
self.shutdown_request(request) What do you think about this solution?
For example, a possible workaround for if server_cls is WSGIServer:
def handle_error(self, *args, **kwargs):
if sys.exc_info()[0] == KeyboardInterrupt:
raise
else:
return super(WSGIServer, self).handle_error(*args, **kwargs)
server_cls.handle_error = handle_error This is how the class class WSGIRefServer(ServerAdapter):
def run(self, app): # pragma: no cover
from wsgiref.simple_server import make_server
from wsgiref.simple_server import WSGIRequestHandler, WSGIServer
import socket
class FixedHandler(WSGIRequestHandler):
def address_string(self): # Prevent reverse DNS lookups please.
return self.client_address[0]
def log_request(*args, **kw):
if not self.quiet:
return WSGIRequestHandler.log_request(*args, **kw)
handler_cls = self.options.get('handler_class', FixedHandler)
server_cls = self.options.get('server_class', WSGIServer)
if server_cls is WSGIServer:
def handle_error(self, *args, **kwargs):
if sys.exc_info()[0] == KeyboardInterrupt:
raise
else:
return super(WSGIServer, self).handle_error(*args, **kwargs)
server_cls.handle_error = handle_error
if ':' in self.host: # Fix wsgiref for IPv6 addresses.
if getattr(server_cls, 'address_family') == socket.AF_INET:
class server_cls(server_cls):
address_family = socket.AF_INET6
self.srv = make_server(self.host, self.port, app, server_cls,
handler_cls)
self.port = self.srv.server_port # update port actual port (0 means random)
try:
self.srv.serve_forever()
except KeyboardInterrupt:
self.srv.server_close() # Prevent ResourceWarning: unclosed socket
raise |
I really hesitate to merge a workaround (#1098) for an obvious bug in a different library. The core problem is that wsgiref catches and ignores KeyboardInterrupt in the main thread. Did you file a bug against python? What did they say? |
I had some issues using reloader option. I am still trying to discover what happens. I have been reading the bottle.py code and other codes for reloading like these ones:
https://github.com/pallets/werkzeug/blob/master/werkzeug/_reloader.py
https://github.com/django/django/blob/master/django/utils/autoreload.py
https://github.com/lepture/python-livereload/blob/master/livereload/watcher.py
https://github.com/gorakhargosh/watchdog
One potential drawback that I see in the current implementation is that is watching all the python modules, including the modules in the standard library.
One possible idea is to filter the modules in the standard library:
Source:
https://stackoverflow.com/questions/22195382/how-to-check-if-a-module-library-package-is-part-of-the-python-standard-library
Another possible idea is to watch changes in a directory, like here:
https://github.com/lepture/python-livereload/blob/master/livereload/watcher.py
One possible improvement that is using django is to create a set of loaded modules, instead of a list, like this:
Bottle is doing this:
Another possible improvement is to use
watchdog
orpyinotify
if they are already installed. Otherwise to use the current implementation as a fallback.Django is providing a customized solution just in case
pyinotify
is not installed. Otherwise it usespyinotify
.Werkzeug is using a similar implementation to the current solution of
bottle.py
and it provides also a solution usingwatchdog
:https://github.com/pallets/werkzeug/blob/master/werkzeug/_reloader.py
Werkzeug also does this extra check:
The text was updated successfully, but these errors were encountered: