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

delete keys when deleting backup versions #6253

Merged
merged 5 commits into from
Oct 25, 2019

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Oct 25, 2019

No description provided.

@uhoreg uhoreg requested a review from a team October 25, 2019 01:31
@uhoreg
Copy link
Member Author

uhoreg commented Oct 25, 2019

sytest failure is:

not ok 383 Gapped incremental syncs include all state changes

which seems unrelated to my changes

Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

LGTM other than tests

}


class E2eRoomKeysHandlerTestCase(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Can you use HomeserverTestCase please? Its the new (ish) way of doing things such that we don't need to spin up a full reactor, and has a number of advantages.

Broadly, it works in the same way instead of doing @defer.inlineCallbacks and yield you can do self.get_success(self.store.some_deferred_returning_func())

@uhoreg
Copy link
Member Author

uhoreg commented Oct 25, 2019

Given that I only made changes the unit test, and the sytest is now passing, it sounds like the "Gapped incremental syncs include all state changes" test is flaky.

@uhoreg uhoreg requested a review from erikjohnston October 25, 2019 15:16
@uhoreg uhoreg merged commit da78f61 into develop Oct 25, 2019
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'da78f6177':
  remove unneeded imports
  switch to using HomeserverTestCase
  remove some unnecessary lines
  add changelog
  delete keys when deleting backups
@DMRobertson DMRobertson deleted the uhoreg/e2e_backup_delete_keys branch June 28, 2022 11:19
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