-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Module reloading fix. #259
Changes from 2 commits
e878b83
a2af428
b5b42a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,7 +118,7 @@ def contains(self, key): | |
|
||
def setup(self): | ||
stderr("\nWelcome to Willie. Loading modules...\n\n") | ||
self.callables = {} | ||
self.callables = set() | ||
|
||
filenames = self.config.enumerate_modules() | ||
# Coretasks is special. No custom user coretasks. | ||
|
@@ -152,16 +152,55 @@ def setup(self): | |
|
||
self.bind_commands() | ||
|
||
@staticmethod | ||
def is_callable(obj): | ||
"""Return true if object is a willie callable. | ||
|
||
Object must be both be callable (function or object with __call__) and | ||
have hashable. Furthermore, it must have either "commands" or "rule" | ||
as attributes to mark it as a willie callable. | ||
""" | ||
if not hasattr(obj, '__call__') or not hasattr(obj, '__hash__'): | ||
# There might not be any objects with __hash__ but no __call__, | ||
# but I'm not sure so check both. __hash__ is required for set. | ||
return False | ||
if hasattr(obj, 'commands') or hasattr(obj, 'rule'): | ||
return True | ||
return False | ||
|
||
def register(self, variables): | ||
""" | ||
With the ``__dict__`` attribute from a Willie module, update or add the | ||
trigger commands and rules to allow the function to be triggered. | ||
""" | ||
# This is used by reload.py, hence it being methodised | ||
for name, obj in variables.iteritems(): | ||
if hasattr(obj, 'commands') or hasattr(obj, 'rule'): | ||
full_name = '%s.%s' % (variables['__name__'], name) | ||
self.callables[full_name] = obj | ||
for obj in variables.itervalues(): | ||
if self.is_callable(obj): | ||
self.callables.add(obj) | ||
|
||
def unregister(self, callables): | ||
"""Unregister all callables and their bindings. | ||
|
||
When unloading a module, this ensures that the unloaded modules will | ||
not get called and that the objects can be garbage collected. Objects | ||
that have not been registered are ignored. | ||
|
||
Args: | ||
callables -- A list of callable objects from a willie module. | ||
""" | ||
def remove_func(func, commands): | ||
"""Remove all traces of func from commands.""" | ||
for func_list in commands.itervalues(): | ||
if func in func_list: | ||
func_list.remove(func) | ||
|
||
for obj in callables.itervalues(): | ||
if not self.is_callable(obj): | ||
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. You already made sure above that anything in callables is callable, so this check is unnecessary. 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. Yes, but the interface for bot.register accepts non-callables, so I figured that for symmetry unregister should too. Are you suggesting removing the check? If there are non-callables, they have to be filtered out. If there are none, checking twice is no big deal. 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. that 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. You're right, I misread that. I might recommend renaming the parameter, though. Chances are I'll misread it again. (Maybe |
||
continue | ||
if obj in self.callables: | ||
self.callables.remove(obj) | ||
for commands in self.commands.itervalues(): | ||
remove_func(obj, commands) | ||
|
||
def bind_commands(self): | ||
self.commands = {'high': {}, 'medium': {}, 'low': {}} | ||
|
@@ -186,7 +225,7 @@ def sub(pattern, self=self): | |
pattern = pattern.replace('$nickname', r'%s' % re.escape(self.nick)) | ||
return pattern.replace('$nick', r'%s[,:] +' % re.escape(self.nick)) | ||
|
||
for func in self.callables.itervalues(): | ||
for func in self.callables: | ||
if not hasattr(func, 'priority'): | ||
func.priority = 'medium' | ||
|
||
|
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.
Python defines the built-in
callable()
function to determine if something is callable. Additionally, callables are already hashable, so there's no need to check for that.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.
I remembered that callable was deprecated, but it has been re-added in python 3.2.
I admin checking for hashable is paranoid, but theoretically someone could remove hash from a function. If you think that's too much I'm fine with it. Also, dict and list have hash but its None, so this check doesn't even work.
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.
I don't think we need to check for
__hash__
.