Skip to content

[Mailer] Improve documentation for mailer failover and round-robin transports #14957

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

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

pableu
Copy link
Contributor

@pableu pableu commented Feb 11, 2021

I think the current documentation for the failover transport is not correct.

If the sending fails, the mailer won’t retry it with the other transports

This isn't what really happens. In my tests (and from studying the code), it seems that the mailer simply retries with the next available transport.

This is also what one would expect from a failover. Failing the first time something goes wrong isn't really a failover ;-)

So the only difference between failover and round-robin is that the latter selects a transport at random. My proposed change reflects that more clearly.

fabpot
fabpot previously approved these changes Feb 12, 2021
@pableu
Copy link
Contributor Author

pableu commented Feb 12, 2021

Updated. Let me know what you think.

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

For me this is more understandable, thank you

@carsonbot carsonbot changed the title Improve documentation for mailer failover and round-robin transports [Mailer] Improve documentation for mailer failover and round-robin transports Feb 15, 2021
@javiereguiluz
Copy link
Member

Pablo, thanks for improving this article. Things are now much easier to understand!

@javiereguiluz javiereguiluz merged commit e73f978 into symfony:4.4 Feb 15, 2021
@pableu pableu deleted the mailer-failover branch February 15, 2021 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants