feat: replace message_ttl with static max retry count #2638
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2638 +/- ##
==========================================
- Coverage 75.28% 75.23% -0.05%
==========================================
Files 295 295
Lines 16627 16672 +45
==========================================
+ Hits 12517 12544 +27
- Misses 3157 3170 +13
- Partials 953 958 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
courier/courier_dispatcher.go
Outdated
| maxRetries := c.deps.CourierConfig(ctx).CourierMessageRetries() | ||
|
|
||
| for k, msg := range messages { | ||
| if time.Now().After(msg.CreatedAt.Add(ttl)) { | ||
| if msg.SendCount >= maxRetries { |
There was a problem hiding this comment.
How can we test that emails won't get send that reach the maximum retry count?
There was a problem hiding this comment.
That is the existing test. Since messages that have the Abandoned state are no longer queued, they won't be sent out. And they receive that state if the condition msg.SendCount > maxRetries is true.
I would like to have a mock SMTP server here, so we can test this case, because currently we are relying on the fact that no SMTP server is running at baz.com which is not ideal. WDYT about that?
There was no documentation about this, I'll add some for the new config. |
|
I created a PR on the docs repo: ory/docs#941 |
|
I moved the message abandonment logic to the appropriate method for this to work in cloud as well, as discussed, and restructured the tests a bit. Are there tests in cloud that ensure that that's the case? |
This PR replaces the `courier.message_ttl` configuration option with a `courier.message_retries` option to limit how often the sending of a message is retried before it is marked as `abandoned`. BREAKING CHANGES: This is a breaking change, as it removes the `courier.message_ttl` config key and replaces it with a counter `courier.message_retries`. Closes ory#402 Closes ory#1598
This PR replaces the
courier.message_ttlconfiguration option with acourier.message_retriesoption to limit how often the sending of a message is retried before it is marked asabandoned.This is a breaking change, as it removes the
message_ttlconfig key.Related issue(s)
#402 #1598
Checklist
If this pull request addresses a security. vulnerability,
I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
Further Comments