Open Vanuan opened 12 years ago
I hope you don't mind if I used portions of your code in my similar project: https://github.com/Vanuan/module_reloader
Here are the changes I've made to also reload dependants:
https://github.com/Vanuan/module_reloader/commit/a7dfb6a79629bcc85b44a0c5b03d24fed5493b76
The differences of my project from yours are:
I think I must be missing some import detail, but I believe the code is working as you suggest it should. To borrow your example:
# dependent.py
print __name__
# dependency.py
import dependency
print __name___
And then at the top-level scope:
import reloader
reloader.enable()
import dependency
reloader.reload(dependency)
... which prints:
dependent
dependency
dependent
dependency
I think you confused dependant and dependency. Dependant is something that depends on (imports) other modules. Dependency is something other modules (dependants) depend on (import). So if we reload dependency, dependant should be reloaded after dependency. As you showed yourself dependant is reloaded before dependency (print statement executed before dependency imported).
Recheck your example please. Make sure that modules are properly named. Your example just can't work (or you're not posting the complete code). As far as you import only dependency, reloader has no way to know that dependant depends on it (and it shouldn't have reloaded dependant).
Probably I wasn't clear enough. Sorry about that. The correct example would be this:
# dependency.py
print __name__
# dependant.py
import dependency
print __name__
And then at the top-level scope:
import reloader
reloader.enable()
import dependant
import dependency
reloader.reload(dependency)
Expected behavior (notice that dependency should be reloaded first):
dependency
dependant
dependency
dependant
Actual behavior (dependant is not reloaded at all):
dependency
dependant
dependency
I expanded this a bit to see if I could sort it out (I couldn't). Here's a more verbose test that defines a list called d in the dependency which is modified by main.py as well as dependent.py, then rechecked by main.py after the reload.
# dependency.py print name
d = []
def add_to_d(item): d.append(item) print "d is %s" % d
# dependent.py print "starting %s..." % name from dependency import add_to_d, d add_to_d('added by %s' % name) print "finished %s." % name
# main.py import reloader reloader.enable()
import dependent import dependency
dependency.add_to_d('added by Main')
print "Reloading..." reloader.reload(dependency) dependency.add_to_d('added my Main') print "Reload done."
print 'dependency.d = %s' % dependency.d print 'dependent.d = %s' % dependent.d
My result is (OSX, Python 2.7):
$ python main.py starting dependent... dependency d is ['added by dependent'] finished dependent. d is ['added by dependent', 'added by Main'] Reloading... dependency d is ['added my Main'] Reload done. dependency.d = ['added my Main'] dependent.d = ['added by dependent', 'added by Main']
It seems clear that dependent.py never reloaded and subsequently has an old reference to d.
Ian E.
On Tue, Nov 13, 2012 at 3:52 AM, John Yani notifications@github.com wrote:
Probably I wasn't clear enough. Sorry about that. The correct example would be this:
dependency.py
print name
dependant.py
import dependency print name
And then at the top-level scope:
import reloader reloader.enable()
import dependant import dependency
reloader.reload(dependency)
Expected behavior (notice that dependency should be reloaded first):
dependency dependant dependency dependant
Actual behavior (dependant is not reloaded at all):
dependency dependant dependency
— Reply to this email directly or view it on GitHubhttps://github.com/jparise/python-reloader/issues/14#issuecomment-10323730.
This email is intended for the use of the individual addressee(s) named above and may contain information that is confidential, privileged or unsuitable for overly sensitive persons with low self-esteem, no sense of humor or irrational religious beliefs. If you are not the intended recipient, any dissemination, distribution or copying of this email is not authorized (either explicitly or implicitly) and constitutes an irritating social faux pas. Unless the word absquatulation has been used in its correct context somewhere other than in this warning, it does not have any legal or grammatical use and may be ignored. No animals were harmed in the transmission of this email, although the yorkshire terrier next door is living on borrowed time, let me tell you. Those of you with an overwhelming fear of the unknown will be gratified to learn that there is no hidden message revealed by reading this warning backwards, so just ignore that Alert Notice from Microsoft: However, by pouring a complete circle of salt around yourself and your computer you can ensure that no harm befalls you and your pets. If you have received this email in error, please add some nutmeg and egg whites and place it in a warm oven for 40 minutes. Whisk briefly and let it stand for 2 hours before icing.
So, do you agree that dependant should be reloaded? Now check what happens if we reload only dependant. You'll see that reloading dependency is not necessary.
Yes absolutely. I expected dependent.py to be reloaded - I thought that's what the reloader library did. At this point, either I have a misunderstanding of this library or there's a bug. If I change the code to reload(dependent), it functions correctly - dependency reloads, then dependent reloads and all the references are correct. However, that seems a bit backwards because if I need to reload the dependency, I reload dependent?
Regardless, reload() is performing exactly as its docstring indicates: Reload an existing module.
Any known dependencies of the module will also be reloaded.
I suppose we're thinking of a complementary function that reloads any dependents of the module for a proper reload.
Ian E.
On Tue, Nov 13, 2012 at 11:59 PM, John Yani notifications@github.comwrote:
So, do you agree that dependant should be reloaded? Now check what happens if we reload only dependant. You'll see that reloading dependency is not necessary.
— Reply to this email directly or view it on GitHubhttps://github.com/jparise/python-reloader/issues/14#issuecomment-10357969.
This email is intended for the use of the individual addressee(s) named above and may contain information that is confidential, privileged or unsuitable for overly sensitive persons with low self-esteem, no sense of humor or irrational religious beliefs. If you are not the intended recipient, any dissemination, distribution or copying of this email is not authorized (either explicitly or implicitly) and constitutes an irritating social faux pas. Unless the word absquatulation has been used in its correct context somewhere other than in this warning, it does not have any legal or grammatical use and may be ignored. No animals were harmed in the transmission of this email, although the yorkshire terrier next door is living on borrowed time, let me tell you. Those of you with an overwhelming fear of the unknown will be gratified to learn that there is no hidden message revealed by reading this warning backwards, so just ignore that Alert Notice from Microsoft: However, by pouring a complete circle of salt around yourself and your computer you can ensure that no harm befalls you and your pets. If you have received this email in error, please add some nutmeg and egg whites and place it in a warm oven for 40 minutes. Whisk briefly and let it stand for 2 hours before icing.
@ianepperson I'm sorry, I was thinking I'm talking to @jparise
At this point, either I have a misunderstanding of this library or there's a bug
Yeap, there's a huge bug. Library does the reverse of what you expect it to do.
Regardless, reload() is performing exactly as its docstring indicates: Reload an existing module. Any known dependencies of the module will also be reloaded.
Yes, this library's reload
function reloads a module and its dependencies.
But if you think about it a bit, you'll see that if you change a module, you don't have to reload its dependencies.
The modules that depend on it should be reloaded.
If you reload dependencies and dependants, you'll eventually reload ALL the modules.
So the library should be changed to reload only dependants.
Say, your module imports os
, why the hell do you need to reload os
module?
I think there's just a mismatch between your expectation of what this library does and its intention. The scenario that the library addresses is that your code imports some module
which, in turn, imports a submodule
. If you then do some work on the submodule
, your code is able to get those changes by running reload(module)
.
If you could only reload dependents and not dependencies, then you would have to call reload(module.submodule)
from your code. But then you have to know exactly what's been changed. Reloading all the dependencies accounts for arbitrary changes in the submodules.
Being able to reload all the dependants makes sense, but I would call this an extension rather than a bug.
For your os
example. there's a blacklist command that prevents reloader from reloading it.
This is a valuable dialogue. Thanks to all of you who are participating!
I agree with @micramm's summary of the situation, but I also acknowledge that my article makes reference to this "missing" feature. If I recall correctly, that particular point is an addition to my original draft of the article upon which the code is based.
I am totally open to supporting this new use case (i.e. reloading dependents), although I've been a bit short on time recently. So please continue to discuss and propose specific changes, and know that I'll continue to follow this discussion and respond as soon as I'm able.
The real problem comes with from package import module
statement.
http://docs.python.org/2/library/functions.html#__import__
the statement
from package import dependant
results in_temp = __import__('package', globals(), locals(), ['dependant'], -1) module = _temp.module
I.e. in our import routine, we don't know what dependency
's "real" parent module is.
As result our dependency tree looks like this:
{ "package" : [module 'package.dependency'] }
So that when we reload dependant:
reloader.reload(dependant)
dependency is not reloaded. But now our dependency looks like this:
{ "package" : [module 'package.dependency'] }
{ "package.dependant" : [module 'package.dependency'] }
So the second time it is reloaded as it's supposed to.
Probably it's a separate bug.
If you then do some work on the submodule, your code is able to get those changes by running reload(module).
From that point of view, yes.
But (for me) the real use-case is to run the reloader thread in the background and track which modules have been modified to reload only those and its dependants.
I think there's just a mismatch between your expectation of what this library does and its intention.
According to you, README is lying:
this reloader will reload the requested module and all other modules that are dependent on that module.
There's no mention of dependencies.
I agree with Vanuan. Below is my dynmodimp module to reload user code / modules from the google app engine database. To reload (after an online edit or upload) I use PEP 302. See the code below. To know the dependencies I was inspired by Jon Parise. Thanks. The user modules in the database, use a fake package : dyn_imports. So we only have to look at modules in this " dyn_imports package" and do not reload system modules. There is no need for it.
To start I use this line of code in appengine_config.py and my webapp2 request handlers init.
builtins.__import__ = dynmodimp.Importer._import
class Importer(object): # __import__ processing
_parent = None # parent module global to check dyn_imports usage
DynModImporter.load_module('dyn_imports') # init PEP 302 hook and pre-load fake module
sys.meta_path = [DynModImporter] # register PEP 302 hook in meta_path
_baseimport = builtins.__import__ # set __import__ builtin hook
@classmethod # __import__ override, must be activated
def _import(cls, mod_name, globs=None, locs=None, fromlist=None, level=-1): # class activated in appengine_config and webapp2 request handler __init__
try :
imp.acquire_lock()
parent = cls._parent # parent global, used for recursive imports
cls._parent = mod_name # next parent if parent has imports
try : # Perform the actual import; results in recursive import if mod_name has to import modules
mod = cls._baseimport(mod_name, globs, locs, fromlist, level) # first using the base import function.
except ImportError :
mod = None # if mod_name in ...runtime or python core
# if we have a parent, we check if this parent imports dyn_imports modules. In that case we also have to reload the parent
if parent is not None and mod_name.split('.')[0] == 'dyn_imports' : # and hasattr(mod, '__file__')
DynModImporter.add_imported_by(mod_name, parent) # add parent to imported_by_list for this mod_name
cls._parent = parent # Lastly, we always restore our global _parent pointer.
return mod
finally :
imp.release_lock()
Below is the PEP302 loader. App engine will start multiple instances based on the load and the app configuration. Every instance uses threading to handle multiple requests. When a module needs to be reloaded, we have to signal (using a flag in memcache) the other instances, to also reload the module. Every instance uses check_and_reload() to check the reload flag in memcache.
To reload a module after an edit or upload of the code : mod_force_reload is used.
class DynModImporter(object): # PEP 302 finder and loader for dyn_imports fake package
_dyn_imports = ['dyn_imports'] # cache mod_names of dyn imports
_imported_by = {} # imported_by list per mod_name
_dyn_reloads_dt = {} # reload date time stamp per mod_name
_instance = str(datetime.datetime.utcnow()) # unique app instance_id
_lock = threading.Lock()
@classmethod
def find_module(cls, mod_name, path=None): # lookup dyn_imports module
if mod_name.split('.')[0] != 'dyn_imports' :
return None
elif mod_name in cls._dyn_imports : # found, already loaded
return cls
elif models.Dynmods.dmod_key_find(mod_name) : # found, and load it
return cls
return None
@classmethod
def load_module(cls, mod_name) :
try :
imp.acquire_lock()
mod = imp.new_module(mod_name)
mod.__loader__ = cls
if mod_name != 'dyn_imports' :
dmod = models.Dynmods.dmod_get_by_key_name(mod_name) # load from DB
mod.__file__ = mod_name
exec dmod.code_object in mod.__dict__
logging.info('dyn_imports hook load or reload : ' + mod_name)
cls._dyn_imports.append(mod_name)
else: # load fake module
mod.__file__ = "[fake module %r]" % mod_name
mod.__path__ = []
sys.modules[mod_name] = mod
return mod
finally :
imp.release_lock()
@classmethod
def mod_force_reload(cls, mod_name): # reload forced by CMS or other instance
with cls._lock :
def _reload(_mod_name, reloaded = None) : # reload mod_name
if _mod_name in sys.modules :
reloaded = reload(sys.modules[_mod_name])
cls._dyn_reloads_dt[_mod_name] = datetime.datetime.utcnow() # date time stamp reload for this instance
logging.warning('re-loaded : ' + _mod_name)
if _mod_name in cls._imported_by : # if mod_name has a import_by_list
for each in cls._imported_by[_mod_name] : # recursive reload imported_by_list
_reload(each)
return reloaded
if _reload(mod_name) :
flag_dyn_reloads = memcache.get('flag_dyn_reloads') # signal other instances to reload mod_name + Imported_by_list
if not flag_dyn_reloads : flag_dyn_reloads = {} # flag dyn_reload for mod_name at datetime
flag_dyn_reloads[mod_name] = {'setter_instance' : cls._instance, 'dyn_reload_dt' : datetime.datetime.utcnow()}
memcache.set('flag_dyn_reloads', flag_dyn_reloads) # set flag
@classmethod
def check_and_reload(cls, flag_dyn_reloads): # used by other instances
with cls._lock :
logging.info('check_and_reload flag found in local instance id : %s' %(cls._instance))
for mod_name in flag_dyn_reloads : # check flag entries, key = mod_name
if flag_dyn_reloads[mod_name]['setter_instance'] != cls._instance : # if this instance is other instance
if mod_name in cls._dyn_reloads_dt and flag_dyn_reloads[mod_name]['dyn_reload_dt'] > cls._dyn_reloads_dt[mod_name] :
cls.mod_force_reload(mod_name) # we have to force a reload
cls._dyn_reloads_dt[mod_name] = datetime.datetime.utcnow() # date time stamp for last reload in this instance
logging.warning('reload : %s, flagged by setter instance %s'
%(mod_name, str(flag_dyn_reloads[mod_name]['setter_instance'])))
@classmethod
def add_imported_by(cls, mod_name, imported_by): # mod_name imported_by imported_by_list
with cls._lock :
imported_by_list = cls._imported_by.setdefault(mod_name, []) # add imported_by to imported_by_list
if imported_by not in imported_by_list :
imported_by_list.append(imported_by)
def get_imported_by_list(mod_name = None): # helper : list imported_by_list for mod_name
if not mod_name :
return DynModImporter._imported_by
else :
return DynModImporter._imported_by.get(mod_name, None) # example: get_imported_by_list('dyn_imports.......')
The code works fine. I'am still testing it, checking if the code is threadsafe.
To be complete, here is the start of the dynmodimp module
#!/usr/bin/python
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
import models, logging, threading, __builtin__ as builtins
import sys, imp, datetime
from google.appengine.api import memcache
And the model :
class Dynmods(db.Model): # key : name
name = db.StringProperty()
bytecode = db.BlobProperty()
@classmethod
def dmod_get_by_key_name(cls, key_name):
dmod = cls.get_by_key_name(key_name)
if dmod :
dmod.key_name = key_name
dmod.code_object = marshal.loads(db.Blob(dmod.bytecode))
return dmod
@classmethod
def dmod_key_find(cls, key_name):
return cls.get_by_key_name(key_name) != None
@classmethod
def dmod_new(cls, key_name, code_object):
return cls(key_name = key_name, name = key_name, bytecode = db.Blob(marshal.dumps(code_object)))
def dmod_put(self):
self.put()
Consider this example:
Module
dependant
depends on moduledependency
. (each module prints__name__
)If we reload dependant:
the dependant module won't be reloaded. It will print:
but it should print:
In your blog post http://www.indelible.org/ink/python-reloading/:
dependency
isA
dependant
isB
Quoting:
But your code clearly doesn't do that.
Is there something I misunderstood?