[Mailer] Pass the port from the Mailgun DSN to the MailgunSmtpTransport #49768
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
So the default should be 465 oder 587 as stated by the mailgun docs? |
|
As far as I know, 587 is the default they're recommending now. From previous discussion on this topic, I gathered that there is intention to change this default in Symfony though, since 465 still works. |
|
I wonder if we should use 567 as default then 🧐 |
|
The idea of these specific DSN is to make the configuration automatic. So, if 587 is the preferred port and if there is no downsides in using it, let's just change the default. |
|
And until now noone is blocked by upstream, at least we got no bug report. |
This was referenced Mar 25, 2023
Add note about custom port being ignored by specific mailer transport DSN
symfony/symfony-docs#18118
Merged
|
In that case, this can be closed in favor of #49805. I've sent a docs PR to clarify the port-ignoring part of the automatic DSNs. |
|
Closing in favor of #49805 |
fabpot
added a commit
that referenced
this pull request
Mar 25, 2023
This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [Mailer] Update default Mailgun port | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT Alternative to #49768. Instead of passing the port from the DSN, update the default to 587, which is recommended by Mailgun. I've also opened a docs PR to clarify that specific bridge DSNs ignore the port: symfony/symfony-docs#18118 Commits ------- 05e9be7 [Mailer] Update default Mailgun port
fabpot
pushed a commit
to sanderdlm/symfony-docs
that referenced
this pull request
Mar 25, 2023
…iler transport DSN See symfony/symfony#49768
fabpot
added a commit
to symfony/symfony-docs
that referenced
this pull request
Mar 25, 2023
…ailer transport DSN (dreadnip) This PR was submitted for the 6.3 branch but it was merged into the 5.4 branch instead. Discussion ---------- Add note about custom port being ignored by specific mailer transport DSN See symfony/symfony#49768 Seems like there has been some confusion about this: * symfony/symfony#46979 * symfony/symfony#44794 People start with the default they're given by the docs, e.g. `sendgrid+smtp` and then try to append `:<port>` to configure it, which doesn't work. As far as I know, the official recommendation is to use the regular `smtp` transport instead when using custom config: symfony/symfony#36224 (comment) Commits ------- b39027f Add note about custom port configuration being ignored by specific mailer transport DSN
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the hardcoded port on the Mailgun SMTP transport and replaces it by the value set in the DSN. The original (465) is still the fallback in case
nullis passed.In the Mailer docs, the following DSN is given as a starting point to use the Mailgun bridge
Developers might want to tweak this DSN, for example to set the region and/or port:
But the port is currently ignored after parsing the DSN.
The current work-around is to not use the
mailgun+smtptransport and stick to the defaultsmtp://one:This is confusing and having the
mailgun+smtpDSN parse the port correctly is the behavior I would expect from reading the docs.References: