Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Split purge API into events vs state and add PurgeEventsStorage#6295

Merged
erikjohnston merged 14 commits intodevelopfrom
erikj/split_purge_history
Nov 8, 2019
Merged

Split purge API into events vs state and add PurgeEventsStorage#6295
erikjohnston merged 14 commits intodevelopfrom
erikj/split_purge_history

Conversation

@erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Oct 30, 2019

This is a bit big as we needed to split out the purge functions to work on events and state in separate transactions.

(Split out from #6245.)

@erikjohnston erikjohnston requested a review from richvdh October 31, 2019 16:00
This does mean that we won't clean up orphaned state groups (i.e. state
groups that were persisted but the associated event wasn't).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erm, haven't we just deleted all the relevant rows from events?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah. And the tests were broken.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

And fix the tests to actually test that things got deleted.
@erikjohnston erikjohnston requested a review from richvdh November 6, 2019 17:03
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it looks plausible now, but your CI is borked

@erikjohnston
Copy link
Member Author

Oh, Collection isn't available on 3.5?

@richvdh
Copy link
Member

richvdh commented Nov 6, 2019

it's in typing_extensions

@erikjohnston
Copy link
Member Author

Hmm, I don't see it in typing_extensions anywhere?

@richvdh
Copy link
Member

richvdh commented Nov 6, 2019

gah. seems you're right. Maybe we could define our own on 3.5. This is starting to get out of scope though.

@erikjohnston erikjohnston merged commit f713c01 into develop Nov 8, 2019
@erikjohnston
Copy link
Member Author

I've just moved it into the docstring for now until we figure out what we're doing :)

@erikjohnston erikjohnston deleted the erikj/split_purge_history branch January 9, 2020 15:47
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'f713c01e2':
  Move type annotation into docstring
  Fix deleting state groups during room purge.
  Use correct type annotation
  Change to not require a state_groups.room_id index.
  Fix up comment
  Update log line to lie a little less
  Add state_groups.room_id index
  Docstrings
  Fix purge room API
  Newsfile
  Split purge API into events vs state
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants