refactor: begin tearing apart AutopushSettings #933
Conversation
pjenvey
added this to the 2017Q2-2.2 Four code refactor issues completed and obsolete legacy code removed... milestone
Codecov Report
@@ Coverage Diff @@
## master #933 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 57 57
Lines 9464 9538 +74
=====================================
+ Hits 9464 9538 +74
Continue to review full report at Codecov.
|
autopush/db.py
Outdated
| def _tomorrow(self): | ||
| return datetime.date.today() + datetime.timedelta(days=1) | ||
|
|
||
| def create_initial_message_tables(self): |
There was a problem hiding this comment.
How come some of these methods have type annotations, but not all of them?
There was a problem hiding this comment.
forgot some of these and we do have some circular imports. this is going to start getting typing.TYPE_CHECKING ugly
| today = datetime.date.today() | ||
| last_month = get_rotating_message_table(self._message_prefix, -1) | ||
| this_month = get_rotating_message_table(self._message_prefix) | ||
| self.current_month = today.month |
There was a problem hiding this comment.
I understand the movement of this code from where it was before, so if a later refactor has this coming that's fine. I assume at some point all initialization stuff like this will occur once (in a single method, so multiple methods aren't setting initial states on self).
There was a problem hiding this comment.
exactly what I'm trying to get at -- I actually did forget that Router/Storage are still being initialized upon creation here. I'll fix this later in another commit. DatabaseManager() creation should be light weight (no network/db calls), until you explicitly tell it to initialize
| @@ -97,3 +97,17 @@ def gauge(self, name, count, **kwargs): | |||
| def timing(self, name, duration, **kwargs): | |||
| self._client.timing(self._prefix_name(name), value=duration, | |||
| host=self._host, **kwargs) | |||
|
|
|||
|
|
|||
| def from_settings(settings): | |||
autopush/router/__init__.py
Outdated
| """Create a dict of IRouters for the given settings""" | ||
| router_conf = settings.router_conf | ||
| routers = dict( | ||
| simplepush=SimpleRouter( |
There was a problem hiding this comment.
Should we make it possible to not enable simplepush?
- move db concerns (incl. metrics) into a simple DatabaseManager. heavier db init's moved into its own setup() method that most tests don't even call - split out routers, now connection doesn't even create them - kill PushState.msg_limit, nor did it need a metrics attrib issue #632
| _message_prefix = attrib(default="message") # type: str | ||
|
|
||
| @classmethod | ||
| def from_settings(cls, settings): |
There was a problem hiding this comment.
we get a light weight __init__ automatically w/ @attrs (some tests use it). these might change around a bit though in a later commit, re the deferred initialization comments above
There was a problem hiding this comment.
Ah! ok, kinda like how we have to have ap_settings.
|
|
||
| import cyclone.web | ||
| from twisted.logger import Logger | ||
| from twisted.python import failure | ||
|
|
||
| if TYPE_CHECKING: # pragma: nocover |
heavier db init's moved into its own setup() method that most tests
don't even call
issue #632