Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mailer] Make RoundRobinTransport retryPeriod configurable #50981

Open
Jean85 opened this issue Jul 14, 2023 · 8 comments · May be fixed by #54939
Open

[Mailer] Make RoundRobinTransport retryPeriod configurable #50981

Jean85 opened this issue Jul 14, 2023 · 8 comments · May be fixed by #54939

Comments

@Jean85
Copy link
Contributor

Jean85 commented Jul 14, 2023

Description

I started using the roundrobin(...) config option of the Mailer, but I'm unable to configure the retryPeriod option, which would grant retries over failed connections:

public function __construct(array $transports, int $retryPeriod = 60)

This is an issue while using the Messenger (which I'm doing, even if not directly) because for 60 seconds my consumer is susceptible to any kind of transport failure.

I tried overriding that, but it required decorating the transport factory and using reflection, so... adding the possibility of piloting that option would be great!

Example

No response

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@Jean85
Copy link
Contributor Author

Jean85 commented Jan 17, 2024

Yes please

@sdespont
Copy link
Contributor

Would be very useful

@xabbuh
Copy link
Member

xabbuh commented Jan 19, 2024

PR welcome

@sdespont
Copy link
Contributor

With something configurable like MAILER_DSN="roundrobin(postmark+api://ID@default sendgrid+smtp://KEY@default)?retry_period=15" ?

@xabbuh
Copy link
Member

xabbuh commented Jan 19, 2024

@sdespont yes

@smnandre
Copy link
Contributor

Is this something that must be in a DSN ?

The retry_period won't change between instances/servers right ?

Should this be configured elsewhere / userland / container ?

@Jean85
Copy link
Contributor Author

Jean85 commented Jan 22, 2024

Yes it should be in DSN IMHO, you should be able to change that in the config without a new commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants