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

Add config to change the delay before sending a notification email #16696

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

MatMaul
Copy link
Contributor

@MatMaul MatMaul commented Nov 27, 2023

Signed-off-by: Mathieu Velten [email protected]

Pull Request Checklist

@github-actions github-actions bot deployed to PR Documentation Preview November 27, 2023 16:58 Active
@MatMaul MatMaul force-pushed the config-email-push-delay branch from 73cb705 to 34f6ee6 Compare November 27, 2023 16:59
@github-actions github-actions bot deployed to PR Documentation Preview November 27, 2023 17:00 Active
@MatMaul MatMaul marked this pull request as ready for review November 27, 2023 17:09
@MatMaul MatMaul requested a review from a team as a code owner November 27, 2023 17:09
@MatMaul
Copy link
Contributor Author

MatMaul commented Nov 28, 2023

I am having a look at the throttle mechanism. I think it doesn't really make sense to have a MIN inferior to the delay, so I am thinking about setting THROTTLE_START_MS equals to the delay.

Copy link
Member

@clokep clokep 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, but a couple small comments.

docs/usage/configuration/config_documentation.md Outdated Show resolved Hide resolved
synapse/config/emailconfig.py Outdated Show resolved Hide resolved
@MatMaul
Copy link
Contributor Author

MatMaul commented Dec 7, 2023

I've addressed the comments, and set THROTTLE_START_MS to be the specified delay instead of being a constant.

@MatMaul
Copy link
Contributor Author

MatMaul commented Dec 7, 2023

We would also need some configuration nobs to tweak throttle max and throttle multiplier, but I was thinking about doing that in another PR. I can add that to this one if you prefer.

@DMRobertson DMRobertson requested a review from a team December 7, 2023 10:55
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.

Thanks @MatMaul!

@erikjohnston erikjohnston merged commit 9f6c644 into matrix-org:develop Dec 12, 2023
40 checks passed
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.

3 participants