-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Store push actions in DB staging area instead of context #2874
Conversation
6491587
to
3a061ca
Compare
* limitations under the License. | ||
*/ | ||
|
||
CREATE TABLE event_push_actions_staging ( |
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.
y'know, you're allowed to put comments in sql files that explain wtf a table is for.
@@ -191,6 +191,9 @@ def action_for_event_by_user(self, event, context): | |||
actions = [x for x in rule['actions'] if x != 'dont_notify'] | |||
if actions and 'notify' in actions: | |||
actions_by_user[uid] = actions | |||
yield self.store.add_push_actions_to_staging( |
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.
the docstring on action_for_event_by_user probably needs updating to say it will do this
hmph. it happened in another commit
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.
indeed we could do with a comment here to explain why we are doing it
Args: | ||
event_id (str) | ||
user_id (str) | ||
actions (list) |
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.
list of ... ?
@@ -751,15 +768,15 @@ def add_push_actions_to_staging(self, event_id, user_id, actions): | |||
Deferred | |||
""" | |||
|
|||
is_highlight = _action_has_highlight(actions) | |||
is_highlight = is_highlight = 1 if _action_has_highlight(actions) else 0 |
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.
too many is_highlight =
.
And it's in the wrong commit :/
the outdated comments are not, in fact, outdated. mostly. |
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.
lgtm
Since push actions for an event can be quite a lot of data, we don't want to have to serialize them when calculating them on a worker. Therefore, we create a staging table and insert the calculated rules in there on the worker, and on the master we simply move the entries across when we persist the event.