Skip to content

Commit

Permalink
Simplify rules manager a bunch
Browse files Browse the repository at this point in the history
Since we're already in an app context, we don't need to re-enter when
loading rules or alerters.
  • Loading branch information
adunham-stripe committed May 28, 2016
1 parent 997976a commit 1f55db5
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 32 deletions.
45 changes: 15 additions & 30 deletions doorman/extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,35 +70,28 @@ def load_alerters(self):
alerters = self.app.config.get('DOORMAN_ALERTER_PLUGINS', {})

self.alerters = {}
with self.app.app_context():
for name, (plugin, config) in alerters.items():
package, classname = plugin.rsplit('.', 1)
module = import_module(package)
klass = getattr(module, classname, None)
for name, (plugin, config) in alerters.items():
package, classname = plugin.rsplit('.', 1)
module = import_module(package)
klass = getattr(module, classname, None)

if klass is None:
raise ValueError('Could not find a class named "{0}" in package "{1}"'.format(classname, package))
if klass is None:
raise ValueError('Could not find a class named "{0}" in package "{1}"'.format(classname, package))

if not issubclass(klass, AbstractAlerterPlugin):
raise ValueError('{0} is not a subclass of AbstractAlerterPlugin'.format(name))
if not issubclass(klass, AbstractAlerterPlugin):
raise ValueError('{0} is not a subclass of AbstractAlerterPlugin'.format(name))

self.alerters[name] = klass(config)
self.alerters[name] = klass(config)

def should_reload_rules(self):
""" Checks if we need to reload the set of rules. """
from doorman.models import Rule

with self.app.app_context():
try:
last_update = Rule.query.order_by(Rule.updated_at.desc()).limit(1).first()
except SQLAlchemyError:
# Ignore DB errors when testing
if not self.app.config['TESTING']:
raise

return False
if self.last_update is None:
return True

if self.last_update < last_update:
newest_rule = Rule.query.order_by(Rule.updated_at.desc()).limit(1).first()
if self.last_update < newest_rule.updated_at:
return True

return False
Expand All @@ -112,15 +105,7 @@ def load_rules(self):
if not self.should_reload_rules():
return

with self.app.app_context():
try:
all_rules = list(Rule.query.all())
except SQLAlchemyError:
# Ignore DB errors when testing
if self.app.config['TESTING']:
all_rules = []
else:
raise
all_rules = list(Rule.query.all())

self.network = Network()
for rule in all_rules:
Expand All @@ -136,7 +121,7 @@ def load_rules(self):
# Note: we do this here, and not in should_reload_rules, because it's
# possible that we've reloaded a rule in between the two functions, and
# thus we accidentally don't reload when we should.
self.last_update = max(all_rules, key=lambda r: r.updated_at)
self.last_update = max(r.updated_at for r in all_rules)

def handle_log_entry(self, entry, node):
""" The actual entrypoint for handling input log entries. """
Expand Down
8 changes: 6 additions & 2 deletions tests/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -1079,11 +1079,15 @@ def test_will_reload_when_changed(self, app, db):
"value": "dummy-query",
}

now = dt.datetime.utcnow()
next = now + dt.timedelta(minutes=5)

# Insert a first rule.
rule = Rule(
name='foo',
alerters=[],
conditions={'condition': 'AND', 'rules': [dummy_rule]}
conditions={'condition': 'AND', 'rules': [dummy_rule]},
updated_at=now
)
db.session.add(rule)
db.session.commit()
Expand All @@ -1100,7 +1104,7 @@ def test_will_reload_when_changed(self, app, db):
# Make a change to a rule.
rule.update(
conditions={'condition': 'OR', 'rules': [dummy_rule]},
updated_at=dt.datetime.utcnow())
updated_at=next)
db.session.add(rule)
db.session.commit()

Expand Down

0 comments on commit 1f55db5

Please sign in to comment.