Skip to content

[Messenger] Fixed check for allowed options in AwsSqs configuration #36873

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
May 22, 2020

Conversation

kroshilin
Copy link
Contributor

@kroshilin kroshilin commented May 19, 2020

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets no
License MIT

Before this fix it was unavailable to create Connection with access_key and secret_key in options, because they were added to $clientConfiguration var, and check for extra options was against $configuration var. Which lead to exception.
The idea is to check input options against self::DEFAULT_OPTIONS (which contains all available options)

Before this fix it was unavailable to create Connection with access_key and secret_key in options, because they were added to $clientConfiguration var, and check for extra options was against $configuration var. Which lead to exception.
The idea is to check input options against self::DEFAULT_OPTIONS (which contains all available options)
@kroshilin kroshilin force-pushed the fix-aws-sqs-extra-options-check branch from f872767 to fb19672 Compare May 19, 2020 14:36
@nicolas-grekas nicolas-grekas added this to the 5.1 milestone May 22, 2020
@nicolas-grekas
Copy link
Member

/cc @jderusse could you please have a look?

Copy link
Member

@jderusse jderusse left a comment

Choose a reason for hiding this comment

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

Indeed, I forgot to take $clientConfiguration options into account.

@chalasr
Copy link
Member

chalasr commented May 22, 2020

Good catch, thanks @kroshilin.

@chalasr chalasr merged commit a7c09ac into symfony:5.1 May 22, 2020
@fabpot fabpot mentioned this pull request May 26, 2020
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