Explicitly mention AWS region

[thePanz]

Q	A
License	MIT
Doc issue/PR	symfony/symfony#33827

[ghost]

Pull request passes validation.

[stof]

@fabpot @Nyholm should we actually build this recipe with separate env variables for the different parts of the DSN, and then combining it in a URL DSN through interpolation ? This is the best way to cause confusion regarding the necessary escaping (as the interpolation cannot apply URL escaping at the time of using the non-URL env variable)

[fabpot]

@stof This has been changed in 4.4. Which means that you (@thePanz) should probably do the same change in the related file in the 4.4 directory.

[thePanz]

@fabpot I am unsure about the 4.4 changes, there the @default is used, should it be replaced with @default?region=eu-west-1?
I will need to have a look at the code first, to find out how the @default is handled

[stof]

@thePanz default is the host in the DSN to tell the transport to use its default host (which depends on the region too). We cannot omit the host in a URL.

[thePanz]

@stof : that's clear. I just wanted to check how the region should be passed to the Factory via the DNS. I found my answer here: https://github.com/symfony/amazon-mailer/blob/4.4/Transport/SesTransportFactory.php#L29

Btw I agree to not use a AWS_SES_REGION variable. I will change my PR soon

[ghost]

Pull request passes validation.

[ghost]

Pull request passes validation.

[ghost]

Pull request passes validation.

[vittominacori]

@thePanz I think that other than requiring it in recipe it should not be set as default null in Transport constructor otherwise users can continue ignore it.
Yes, obviously, it keep back compatibility and everything working but it make sense to keep $this->region = $region ?: 'eu-west-1'; in a constructor having value set as null by default like here?

[fabianoroberto]

@stof : that's clear. I just wanted to check how the region should be passed to the Factory via the DNS. I found my answer here: https://github.com/symfony/amazon-mailer/blob/4.4/Transport/SesTransportFactory.php#L29

Btw I agree to not use a AWS_SES_REGION variable. I will change my PR soon

Why is it better not to use AWS_SES_REGION variable?

IMHO I think is better have an explicit variable empty or with a default value like this:

"env": {
         "#1": "AWS_ACCESS_KEY=",
         "#2": "AWS_SECRET_KEY=",
         "#3": "AWS_SES_REGION="
         "#4": "MAILER_DSN=smtp://$AWS_ACCESS_KEY:$AWS_SECRET_KEY@ses?region=$AWS_SES_REGION",
         "#5": "MAILER_DSN=http://$AWS_ACCESS_KEY:$AWS_SECRET_KEY@ses?region=$AWS_SES_REGION"
     }

So in sample above if AWS_SES_REGION is empty default value will be eu-west-1 ($this->region = $region ?: 'eu-west-1';)

[stof]

@fabianoroberto using other variables interpolated in a URL is a bad idea, because the interpolation will not apply any escaping (it cannot) and so the separate variables will need to contain URL-escaped value (in which case they become unusable on their own).
That's why the AWS_ACCESS_KEY and AWS_SECRET_KEY have been removed in the new version of the recipe.

[fabpot]

merge now