Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
Don't add rejections to the state_group, persist all rejections #948
Conversation
All checks have passed
NegativeMjark
assigned
erikjohnston
Jul 25, 2016
|
Err, i take that back |
NegativeMjark
added some commits
Jul 25, 2016
All checks have passed
All checks have passed
|
I can't speak to the correctness of the changes themselves, but in my experience, when stuff starts getting complicated, lots of comments explaining what is going on can really help. I'm surprised to see quite a lot of code here, with little attention to comments explaining why things are doing what they are doing. |
richvdh
commented on an outdated diff
Jul 26, 2016
| @@ -442,6 +433,9 @@ def _persist_events_txn(self, txn, events_and_contexts, backfilled): | ||
| # Handle the case of the list including the same event multiple | ||
| # times. The tricky thing here is when they differ by whether | ||
| # they are an outlier. | ||
| + if context.rejected: |
richvdh
Member
|
richvdh
commented on an outdated diff
Jul 26, 2016
| @@ -591,10 +557,28 @@ def event_dict(event): | ||
| ], | ||
| ) | ||
| - if context.rejected: | ||
| - self._store_rejections_txn( | ||
| - txn, event.event_id, context.rejected | ||
| - ) | ||
| + to_remove = set() |
richvdh
Member
|
richvdh
commented on an outdated diff
Jul 26, 2016
| @@ -610,6 +594,42 @@ def event_dict(event): | ||
| ], | ||
| ) | ||
| + if event.type == EventTypes.Redaction and event.redacts is not None: |
richvdh
Member
|
richvdh
commented on the diff
Jul 26, 2016
| @@ -610,6 +594,42 @@ def event_dict(event): | ||
| ], | ||
| ) | ||
| + if event.type == EventTypes.Redaction and event.redacts is not None: | ||
| + self._remove_push_actions_for_event_id_txn( | ||
| + txn, event.room_id, event.redacts | ||
| + ) | ||
| + | ||
| + self._store_mult_state_groups_txn(txn, events_and_contexts) | ||
| + | ||
| + self._handle_mult_prev_events( | ||
| + txn, | ||
| + events=[event for event, _ in events_and_contexts], | ||
| + ) | ||
| + | ||
| + for event, _ in events_and_contexts: | ||
| + if event.type == EventTypes.Name: |
richvdh
Member
|
erikjohnston
commented on the diff
Jul 26, 2016
| - elif event.type == EventTypes.Redaction: | ||
| - self._store_redaction(txn, event) | ||
| - elif event.type == EventTypes.RoomHistoryVisibility: | ||
| - self._store_history_visibility_txn(txn, event) | ||
| - elif event.type == EventTypes.GuestAccess: | ||
| - self._store_guest_access_txn(txn, event) | ||
| - | ||
| - self._store_room_members_txn( | ||
| - txn, | ||
| - [ | ||
| - event | ||
| - for event, _ in events_and_contexts | ||
| - if event.type == EventTypes.Member | ||
| - ], | ||
| - backfilled=backfilled, | ||
| - ) |
erikjohnston
Owner
|
|
I guess the main changes here are: don't add rejected events to event forward extremities table (and don#t and them to the push actions). |
|
Although I'm slightly concerned that we have stopped storing various bits of information about the event, and how that affects other storage functions. I'd prefer if, for now, the changes were as minimal as possible so that we can get the fix out and have refactoring (and commenting) as a separate PR. |
NegativeMjark
added some commits
Jul 26, 2016
All checks have passed
NegativeMjark commentedJul 25, 2016
No description provided.