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

Do not call setup_background_tasks twice in tests. #16150

Merged
merged 2 commits into from
Aug 22, 2023
Merged

Conversation

clokep
Copy link
Member

@clokep clokep commented Aug 21, 2023

This gets called anyway via hs.setup():

synapse/synapse/server.py

Lines 346 to 347 in 358896e

if self.config.worker.run_background_tasks:
self.setup_background_tasks()

This is important in that hs.setup() has an extra if-statement which only runs this code if the worker is running background tasks, instead of unconditionally.

Spun out of #16066.

@clokep clokep marked this pull request as ready for review August 21, 2023 17:48
@clokep clokep requested a review from a team as a code owner August 21, 2023 17:48
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Presumably setup_background_tasks should only get run if you're the worker configured to run background tasks.

@clokep
Copy link
Member Author

clokep commented Aug 22, 2023

Seems reasonable. Presumably setup_background_tasks should only get run if you're the worker configured to run background tasks.

Pretty much, yes. 👍

@clokep
Copy link
Member Author

clokep commented Aug 22, 2023

This is hitting a complement flake, but changes unittest only code so I'm going to force-merge.

@clokep clokep merged commit 6d7c63f into develop Aug 22, 2023
35 of 37 checks passed
@clokep clokep deleted the clokep/setup-twice branch August 22, 2023 11:46
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