Skip to content

Loading…

Shell auto-reload #52

Open
wants to merge 3 commits into from

3 participants

@Kazy

As agreed on #51, here is the pull request for adding auto reloading feature to the Python shell with a Flask project.

@Kazy

I forgot ReloadEventHandler was subclassing FileSystemEventHandler. Maybe we should put it in another file, and import it only if watchdog is present ?
Also, it may be better to regroup all of these commits in a single one ?

@techniq
Collaborator

The Django extension sets FileSystemEventHandler to object if watchdog is not available so maybe we should do the same (https://github.com/Bpless/django-extensions/blob/master/django_extensions/management/shells.py#L11)

I just noticed you're also using importlib, which is only available in Python 2.7+, but is available from PyPI for backwards compatibility. This will be another dependency for reload to work and should be added to your try block.

I agree we should rebase the commits before merging.

@Kazy

Changes has been made based on your remarks. For the rebase, since this has already been pushed, will github handle correctly a forced rebase of all of these commits ? I've never done it with a pull request, that's why I'm asking.

@techniq
Collaborator

The Travis tests are still failing for python 2.5 and 2.6 due to a left over import importlib.

Regarding the rebase, you can rebase/force push and git/github should handle it (I've not done it myself yet ;) ). This is a pretty good Github contribution guide talking about rebasing - https://github.com/aurajs/aura/wiki/Git-Guide

@Kazy

Sorry, I wasn't careful enough, I should have run the tests before ! They should pass now.

@techniq
Collaborator

Great. I probably won't get a chance to test it out till tonight, but if all looks good, I'll merge it in.

If you want, go ahead and squash your commits down into a single commit. I can squash it before I merge it in as well.

@techniq
Collaborator

I synced up with your changes and imported it into an existing project I have, but could not get the reload functionality to work. I tried from both bpython (my usual shell) and standard python repl.

Have you been able to get this to successfully work, and if so, any tips on what I could be overlooking?

@Kazy

I just tested it right now, and it works. Be sure watchdog and importlib are available. Also, just before a shell is launched, there is a print with the project root path, the one which is being watch by watchdog. Be sure it corresponds to your project. That's the first two things I have in mind that could solve your problem.

@techniq
Collaborator

I had watchdog (and importlib via Python 2.7) and was seeing the path printed on shell startup (not sure we should print this by default when we merge this in, but it's good for debugging now).

I added a print statement in _reload_module and do see it being triggered, but my class defs does not appear to update.

For example, I'm overriding __str__ in my model and try hard coding a return value (return 'Foo') instead of the regular value (in this case, return self.full_name), which I see triggers the reload, but running str(u) doesn't change anything. I also grab a new instance from the database as I know Python doesn't update instance definitions when monkey patching

>>> u = models.User.get(1)
>>> str(u)
'Admin User'
>>> Reloading
>>> str(u)
'Admin User'
>>> u2 = models.User.get(2)
>>> str(u2)
'Test Admin'
>>> Reloading
>>> str(u2)
'Test Admin'

Maybe this is a SQLAlchemy issue, but I also tried adding a new entry into my config and checking for it (current_app.config['FOO']) but it did not update either (KeyError).

Do you have a use case you are able to have work and I can see if I can reproduce it on my end?

@Kazy

Both cases are working for me.
For the first one, I've used the Flask-SQLAlchemy example from its documentation to test it.

class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    username = db.Column(db.String(80), unique=True)
    email = db.Column(db.String(120), unique=True)

    def __init__(self, username, email):
        self.username = username
        self.email = email

    def __str__(self):
        return '<User %r>' % self.username

Then, in a shell:

>>> a = User.query.get(1)
>>> str(a)
"<User u'admin'>"

I then change the __str__ method to return "Foo", as you did.

    def __str__(self):
        return 'Foo'

and in my shell:

>>> str(a)
'Foo'

I've also been able to update the app.config['foo'] key, even if it didn't exist before. Does the reloading feature works with basic case for you ? Like having a class with a single method return whatever you want. Then in a Python shell, you import this class and instantiate an object. You can check the return value of your method. After that, change your method to return something else, and call it again. The return value should have change without you having to do something.

@techniq
Collaborator

I've been having a lot of issues getting this to work reliably. I created an example app based on the example app from Flask-SQLAlchemy and made a gist - https://gist.github.com/4602100. Here's some of my issues:

Configuration not being re-read
Updating application.cfg will not trigger an update, and updating app.py to trigger an update will not re-read the config. In my production app I tried using this in, I am using classes in a config.py file and also can not get the configuration to be re-read.

Models in separate file throwing Error
I'm able to get a model class to reload in the example app, although when I moved the class definition out of app.py and into models.py, I get the following error:

InvalidRequestError: Table 'todos' is already defined for this MetaData instance.  Specify 'extend_existing=True' to redefine options and columns on an existing Table object.

I didn't get this error when I originally had Todo inside of app.py (where from model import Todo is now).

I'm also having a lot of issues with getting this to work with my production app, which I'll look into more once we get all the example app issues resolved.

Thanks for working on this.

@Kazy

Configuration not being re-read
The reloader only watches .py files, so the reloader will not be triggered if a .cfg is changed. If you modify the file where the configuration is read (app.py in your example), it should be re-read.
I don't know how we could fix that in a clean way, because we have no idea which file is loading the configuration. We could detect change in .cfg files, and maybe call ourselves from Flask-Script the app.config.from-pyfile method. But it may reload incorrect files and do not wanted things. An other solution may be to reload every module each time a .cfg is changed.

Models in separate file throwing Error
To understand correctly your problem, you had your model class in app.py, with a python shell opened and this model imported, and then, without closing the python shell, you've moved the model class definition in a separate file, and then imported it again ?
I have no idea if it works that way or anything, but the class definition may not have been reloaded correctly if there already was an instance of it in your shell. If you instantiate a class, and then remove this class from your file, the variable holding the object still work. It may be a similar case here, with object not being fully unloaded and SQLAlchemy keeping a reference of it.

I will try to fix these issues. The second one may need more work, as it may include some SQLAlchemy specificity here.

@techniq
Collaborator

Configuration not being re-read
I think not watching for .cfg changes but having it reset on app.py change would be fine, although I could not get mine to reload the config (the reload was tripped when I saved app.py, but checking the value current_app.config['FOO'] did not update. Can you test my gist and see if this works for you?

Models in separate file throwing Error
To understand correctly your problem, you had your model class in app.py, with a python shell opened and this model imported, and then, without closing the python shell, you've moved the model class definition in a separate file, and then imported it again ? No, I originally tested having everything in app.py, and was able to perform the __str__ change without issue. I then moved the model definition out of app.py and into a new models.py (trying to represent my production code that has issues with reloading in the shell), and I started getting the already defined for this MetaData instance. error. If you test the app in the gist in it's current state, hopefully you'll see what I mean.

Thanks again.

@Kazy

I've been able to reproduce the second issue. It thinks the way to do it would be to create an new MetaData instance. And I've also found a way to fix this. The problem is that the Todo class is already in the SQLAlchemy MetaData instance, and when reloading the Todo class, it tries to add it, resulting in an error. If we call db.Model.metadata.clear() before that, everything reload correctly (except a warning may occur). I need to test it further, but it seems to be a correct way of doing it.
We also need to see how to automatically clear() and reload every models. Does having to pass the metadata instance (or the BaseClass) and the concerned models to the Shell seems fine to you ? (Shell(sqlalchemy=(metaclass,[model1,model2,model3])), something like that)

For the first one, if I modify the .cfg file, and add/remove something to the app.py file, the application.cfg is correctly reloaded. And watching .cfg files and reloading the app module could do it quite easily I think.

@techniq
Collaborator

I don't like having to pass all the models would be a good idea if we can work around it (it would be hard to manage). Also, I think passing in the metaclass would be fine, but it would be nice if it was optional. We should be able to autodetect it by looking at app.extensions['sqlalchemy'].db.metadata if the they are using Flask-SQLAlchemy. Allowing it to be passed in would allow users who use SQLAlchemy directly to provide their instance. We may need to support special handling of Mongo (and possible others), so I think passing it as sqlalchemy parameter would be a good idea.

I figured out why my config wasn't updating. For some reason looking at current_app.config doesn't reload, but looking at the app instance directly does.

Doesn't reload correctly:

from flask import current_app
current_app.config['FOO']

Reloads correctly:

from app import app
app.config['FOO']

Not sure if there is anything we can do to refresh the current_app instance, and probably not that big of an issue.

@Kazy

There is an other problem with the swapping of SQLAlchemy models: a warning is emitted the first time we reload them. I have to investigate (and probably see with the SQLAlchemy community) how to properly reload those.

@techniq
Collaborator

What's the warning being emitted? Does it have to do with objects being added to the session?

@Kazy

"SAWarning: This declarative base already contains a class with the same class name and module name as models.Todo, and will be replaced in the string-lookup table.
existing.add_item(cls)"
Here is the message we get the first time we do a change (with clear() before doing it). After that, each other change goes smoothly.

@Kazy

Apparently, this issue only happens when the Flask-SQLAlchemy is used and is triggered when the reload function is used with a model class. Using a basic SQLAlchemy setup (create engine, session, using declarative_base()) "fix it" (doesn't cause any problem). I will open an issue about it on Flask-SQLAlchemy's github.

@techniq
Collaborator

That's interesting, let me know if you hear back from them. I know Armin has been awfully busy and hasn't been as attentive to his open source projects. I actually reached out to him about helping maintain Flask-SQLAlchemy and chatted on IRC a few times, but he never got around to giving me push permissions. If you don't hear anything back let me know and I'll try to see where Flask-SQLAlchemy is doing things different from standard SQLAlchemy. Even if we find it, it will probably take a while to get merged in (after one of our chats Armin got quite active on Flask-SQLAlchemy merged/closed a good bit of requests, but there are still 21 open as of today).

@Kazy

I've been these days and I've only been able to fix an issue with Sublime Text 3 (and maybe 2) not triggering the reload mechanism when saving a file. I still have this pull-request in my mind :)

@Kazy

I now ran into an other bug on both SQLAlchemy and Flask-SQLAlchemy, relative to the access of an attribute. After a reload, trying to get an attribute results in:

Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "models.py", line 17, in __repr__
    return '<User {} | Mail {}>'.format(self.username, self.email)
  File "/home/akearr/venv/flask_script/local/lib/python2.7/site-packages/sqlalchemy/orm/attributes.py", line 313, in __get__
    if self._supports_population and self.key in dict_:
  File "/home/akearr/venv/flask_script/local/lib/python2.7/site-packages/sqlalchemy/orm/attributes.py", line 270, in __getattr__
    key)
AttributeError: Neither 'InstrumentedAttribute' object nor 'Comparator' object associated with User.username has an attribute '_supports_population'

and trying to set an attribute in:

Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/home/akearr/venv/flask_script/local/lib/python2.7/site-packages/sqlalchemy/orm/attributes.py", line 302, in __set__
    self.impl.set(instance_state(instance),
AttributeError: 'NoneType' object has no attribute 'set'

The switch of the __class__ attribute on existing instances seems to mess up getter and setter. I have no idea how SQLAlchemy works and the amount of complexity involved makes it hard to figure out what is really going on. The other issue still persists with Flask-SQLAlchemy. And I have no idea how to fix those. I will see if I can get zzzeek on IRC.

@techniq
Collaborator

Awesome, thanks for continuing to look into this.

@davidism davidism referenced this pull request
Open

Shell autoreload #51

@sibeliusseraphini

@Kazy @techniq any progress on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 18, 2013
  1. @Kazy
Commits on Jan 22, 2013
  1. @Kazy
Commits on Mar 9, 2013
  1. @Kazy

    Temp commit

    Kazy committed
Showing with 217 additions and 21 deletions.
  1. +2 −1 .gitignore
  2. +215 −20 flask_script/commands.py
View
3 .gitignore
@@ -12,4 +12,5 @@ dist/
*.swp
*.tox
*.zip
-*.sublime-*
+*.sublime-*
+.idea
View
235 flask_script/commands.py
@@ -1,17 +1,60 @@
# -*- coding: utf-8 -*-
from __future__ import absolute_import
from __future__ import with_statement
+import atexit
+import inspect
import os
import code
import warnings
+import logging
+
import argparse
from flask import _request_ctx_stack
from .cli import prompt, prompt_pass, prompt_bool, prompt_choices
+try:
+ import importlib
+except ImportError:
+ importlib = None
+
+_listener_enabled = False
+FileSystemEventHandler = object
+
+if importlib is not None:
+ try:
+ from watchdog.events import FileSystemEventHandler
+ except ImportError:
+ pass
+
+ try:
+ from watchdog.observers import Observer
+
+ _listener_enabled = True
+ except ImportError:
+ pass
+ else:
+ # Threading object which listens for file system changes
+ # Keep a reference to this object in the global scope
+ # So that we can join it on program exit
+ # Failing to do so raises ugly Threading exceptions
+ # We may choose to just suppress stderr instead
+ # Since waiting for thread to end seems to take time (~1sec)
+ observer_thread = Observer()
+
+ def kill_observer_thread():
+ # Clean up the filesystem listener thread on program exit
+ # This seems to add roughly one second to shut down, so it's not ideal
+ if observer_thread.is_alive():
+ observer_thread.stop()
+ observer_thread.join()
+
+ # Register functions to be called when Python program ends normally
+ atexit.register(kill_observer_thread)
+
class InvalidCommand(Exception):
pass
@@ -45,9 +88,9 @@ def __init__(self, *options, **kwargs):
self.required = kwargs.pop("required", None)
if ((self.title or self.description) and
- (self.required or self.exclusive)):
+ (self.required or self.exclusive)):
raise TypeError("title and/or description cannot be used with "
- "required and/or exclusive.")
+ "required and/or exclusive.")
super(Group, self).__init__(**kwargs)
@@ -120,13 +163,13 @@ def create_parser(self, prog):
if isinstance(option, Group):
if option.exclusive:
group = parser.add_mutually_exclusive_group(
- required=option.required,
- )
+ required=option.required,
+ )
else:
group = parser.add_argument_group(
- title=option.title,
- description=option.description,
- )
+ title=option.title,
+ description=option.description,
+ )
for opt in option.get_options():
group.add_argument(*opt.args, **opt.kwargs)
else:
@@ -191,6 +234,10 @@ class Shell(Command):
:param use_ipython: use IPython shell if available, ignore if not.
The IPython shell can be turned off in command
line by passing the **--no-ipython** flag.
+ :param use_reloader: auto-reload the project if watchdog is available
+ by watching the root_path from app.
+ The auto-reload can be turned off in command
+ line by passing the **-no-reload** flag.
"""
banner = ''
@@ -198,11 +245,11 @@ class Shell(Command):
description = 'Runs a Python shell inside Flask application context.'
def __init__(self, banner=None, make_context=None, use_ipython=True,
- use_bpython=True):
-
+ use_bpython=True, use_reloader=True):
self.banner = banner or self.banner
self.use_ipython = use_ipython
self.use_bpython = use_bpython
+ self.use_reloader = use_reloader
if make_context is None:
make_context = lambda: dict(app=_request_ctx_stack.top.app)
@@ -212,13 +259,17 @@ def __init__(self, banner=None, make_context=None, use_ipython=True,
def get_options(self):
return (
Option('--no-ipython',
- action="store_true",
- dest='no_ipython',
- default=not(self.use_ipython)),
+ action="store_true",
+ dest='no_ipython',
+ default=not (self.use_ipython)),
Option('--no-bpython',
- action="store_true",
- dest='no_bpython',
- default=not(self.use_bpython))
+ action="store_true",
+ dest='no_bpython',
+ default=not (self.use_bpython)),
+ Option('--no-reload',
+ action="store_true",
+ dest='no_reloader',
+ default=not (self.use_reloader))
)
def get_context(self):
@@ -227,7 +278,7 @@ def get_context(self):
"""
return self.make_context()
- def run(self, no_ipython, no_bpython):
+ def run(self, no_ipython, no_bpython, no_reloader):
"""
Runs the shell. If no_bpython is False or use_bpython is True, then
a BPython shell is run (if installed). Else, if no_ipython is False or
@@ -235,17 +286,40 @@ def run(self, no_ipython, no_bpython):
"""
context = self.get_context()
+
+ if not no_reloader:
+ if not _listener_enabled:
+ print("If you want to use the auto-reloading functionality, you need to install watchdog "
+ "(you can do it using pip)")
+ else:
+ autoreload_path = _request_ctx_stack.top.app.root_path
+
+ if not autoreload_path:
+ raise InvalidCommand("To use the auto-reload, the root_path variable of app must be available.")
+ else:
+ print("Using this path as project root: {path}".format(path=autoreload_path))
+
+ def listen_for_changes(shell_globals, project_root):
+ # Begin thread which listens for file system changes via Watchdog library
+ event_handler = ReloaderEventHandler(project_root=project_root, shell_globals=shell_globals)
+ observer_thread.schedule(event_handler, path=project_root, recursive=True)
+ observer_thread.start()
+
+ listen_for_changes(context, autoreload_path)
+
if not no_bpython:
# Try BPython
try:
from bpython import embed
- embed(banner=self.banner, locals_=self.get_context())
+
+ embed(banner=self.banner, locals_=context)
return
except ImportError:
# Try IPython
if not no_ipython:
try:
import IPython
+
try:
sh = IPython.Shell.IPShellEmbed(banner=self.banner)
except AttributeError:
@@ -259,6 +333,127 @@ def run(self, no_ipython, no_bpython):
code.interact(self.banner, local=context)
+class ReloaderEventHandler(FileSystemEventHandler):
+ """
+ Listen for changes to modules within the Flask project
+ On change, reload the module in the Python Shell
+
+ Almost everything here come from
+ https://github.com/Bpless/django-extensions/commit/0078b4a513b0035b97820c0788ba990d20f80d48#L2R142
+ Thanks to him !
+ """
+
+ def __init__(self, *args, **kwargs):
+ self.project_root = kwargs.pop('project_root', None)
+ self.shell_globals = kwargs.pop('shell_globals', None)
+ super(ReloaderEventHandler, self).__init__(*args, **kwargs)
+
+ def dispatch(self, event):
+ event_path = event.src_path
+ path, file_extension = os.path.splitext(event_path)
+ if all([
+ file_extension == '.py',
+ 'shell_plus' not in path,
+ self.project_root in path
+ ]):
+ return super(ReloaderEventHandler, self).dispatch(event)
+
+ def on_created(self, event):
+ super(ReloaderEventHandler, self).on_created(event)
+ self._force_reload(event)
+
+ def on_modified(self, event):
+ """
+ Called by dispatch on modification of file in the Flask project
+ """
+ super(ReloaderEventHandler, self).on_modified(event)
+ self._force_reload(event)
+
+ def _force_reload(self, event):
+ """
+ Reload the altered module
+ """
+ cleaned_path = self._clean_path(event.src_path)
+ path_components = cleaned_path.split(os.path.sep)
+ self._reload_module(path_components)
+
+ def _clean_path(self, path):
+ """Remove the leading project path"""
+ project_root = self.project_root if self.project_root.endswith('/') else "{}/".format(self.project_root)
+ path_from_project_root = path.replace(project_root, '')
+ # Remove trailing ".py" from module for importing purposes
+ return os.path.splitext(path_from_project_root)[0]
+
+ def _reload_module(self, path_components):
+ """
+ Wrapper for __builtin__ reload() function
+ In addition to reloading the module,
+ we reset the associated classes in the global scope of the shell.
+
+ Consequently, we don't have to manaully reimport (i.e. 'from app import MyClass')
+ Instead, MyClass will have changed for us automagically
+
+ More interestingly, we also dynamically update the classes
+ of existing object instances in the global scope with `_update_class_instances`.
+
+ ## In a Shell session
+ obj = MyKLS()
+ obj.getchar() --> 'a'
+
+ ## While still in the Shell,
+ ### We change the function definition of getchar() in the filesytem to return 'b'
+ ### In our Shell, we will see that
+
+ obj.getchar() --> 'b'
+
+ This behavior is very experimental and possibly dangerous but powerful
+ Cuts down time and frustration during pdb debugging
+ """
+
+ # We attempt to import the module from the project root
+ # This SHOULD be agnostic of app/project structure
+ while True:
+ try:
+ module = importlib.import_module('.'.join(path_components))
+ except ImportError:
+ path_components.pop(0)
+ if not path_components:
+ return
+ else:
+ break
+
+ reload(module)
+ # Reload objects into the global scope
+ # This has the potential to cause namespace collisions
+ # The upside is that we don't have to reimport (i.e. from module import ObjName)
+ for attr in dir(module):
+ if (
+ not (attr.startswith('__') and attr.endswith('__'))
+ and self.shell_globals.get(attr)
+ ):
+ self.shell_globals[attr] = getattr(module, attr)
+ self._update_class_instances(module, attr)
+
+ def _update_class_instances(self, module, attr):
+ """
+ Reset the __class__ of all instances whoses
+ class has been reloaded into the shell
+
+ This allows us to do CRAZY things such as
+ effectively manipulate an instance's source code
+ while inside a debugger
+ """
+ module_obj = getattr(module, attr)
+ if inspect.isclass(module_obj):
+ for obj in self.shell_globals.values():
+ # hasattr condition attempts to handle old style classes
+ # The class __str__ check may not be ideal but it seems to work
+ # The one exception being if you changes the __str__ method
+ # of the reloaded object. That edge case is not handled
+ if hasattr(obj, '__class__') and str(obj.__class__) == str(module_obj):
+ obj.__class__ = module_obj
+
+
class Server(Command):
"""
Runs the Flask development server i.e. app.run()
@@ -283,7 +478,6 @@ class Server(Command):
def __init__(self, host='127.0.0.1', port=5000, use_debugger=True,
use_reloader=True, threaded=False, processes=1,
passthrough_errors=False, **options):
-
self.port = port
self.host = host
self.use_debugger = use_debugger
@@ -294,7 +488,6 @@ def __init__(self, host='127.0.0.1', port=5000, use_debugger=True,
self.passthrough_errors = passthrough_errors
def get_options(self):
-
options = (
Option('-t', '--host',
dest='host',
@@ -365,6 +558,7 @@ def handle(self, app, host, port, use_debugger, use_reloader,
class Clean(Command):
"Remove *.pyc and *.pyo files recursively starting at current directory"
+
def run(self):
for dirpath, dirnames, filenames in os.walk('.'):
for filename in filenames:
@@ -378,6 +572,7 @@ class ShowUrls(Command):
"""
Displays all of the url matching routes for the project.
"""
+
def __init__(self, order='rule'):
self.order = order
@@ -387,7 +582,7 @@ def get_options(self):
dest='order',
default=self.order,
help='Property on Rule to order by (default: %s)' % self.order,
- ),
+ ),
return options
Something went wrong with that request. Please try again.