Skip to content

[Mailer] Change DSN syntax #33494

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
Sep 7, 2019
Merged

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Sep 6, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR -

The current syntax for failover and roundrobin is confusing. && and || do not really convey the right meaning. I realized that while working on a new transport that will send on more than one transport in parallel. && would be a natural fit, but that's already taken.

So, this pull request changes the syntax to be more explicit.

@fabpot fabpot changed the title Mailer change dsn syntax [Mailer] Change DSN syntax Sep 6, 2019
@fabpot fabpot force-pushed the mailer-change-dsn-syntax branch 2 times, most recently from c29fc96 to aec11ea Compare September 6, 2019 14:30
@fabpot
Copy link
Member Author

fabpot commented Sep 6, 2019

This PR also allows to nest transports.

@nicolas-grekas nicolas-grekas added this to the next milestone Sep 6, 2019
@fabpot fabpot force-pushed the mailer-change-dsn-syntax branch from aec11ea to 4408910 Compare September 6, 2019 14:41
@stof
Copy link
Member

stof commented Sep 6, 2019

would be great to add tests covering the error cases of the parsing (unclosed parenthesis, nesting a parenthesis directly in the argument, passing an unknown function name, garbage at the end, etc...)

@fabpot
Copy link
Member Author

fabpot commented Sep 7, 2019

@stof I've added some more tests.

@fabpot fabpot force-pushed the mailer-change-dsn-syntax branch from 4408910 to f30dc2b Compare September 7, 2019 06:34
@fabpot fabpot force-pushed the mailer-change-dsn-syntax branch from f30dc2b to 39dd213 Compare September 7, 2019 06:41
fabpot added a commit that referenced this pull request Sep 7, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[Mailer] Change DSN syntax

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | yes
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | -

The current syntax for failover and roundrobin is confusing. `&&` and `||` do not really convey the right meaning. I realized that while working on a new transport that will send on more than one transport in parallel. `&&` would be a natural fit, but that's already taken.

So, this pull request changes the syntax to be more explicit.

Commits
-------

39dd213 [Mailer] Change the syntax for DSNs using failover or roundrobin
@fabpot fabpot merged commit 39dd213 into symfony:4.4 Sep 7, 2019
@fabpot fabpot deleted the mailer-change-dsn-syntax branch September 7, 2019 12:04
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
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.

4 participants