-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] fixed queue_name option on amazon sqs connection #37364
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
[Messenger] fixed queue_name option on amazon sqs connection #37364
Conversation
d2e5518
to
0a6ed8c
Compare
0a6ed8c
to
df7e562
Compare
src/Symfony/Component/Messenger/Bridge/AmazonSqs/Tests/Transport/ConnectionTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Bridge/AmazonSqs/Transport/Connection.php
Show resolved
Hide resolved
54d8124
to
f60068d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
What about adding a test for when you provide both a parameter and add a DSN with a queue name in it?
What should be the preferred result?
new Connection(['queue_name' => 'queue'], new SqsClient(['region' => 'eu-west-1', 'accessKeyId' => null, 'accessKeySecret' => null], null, $httpClient)), | ||
Connection::fromDsn('sqs://default', ['queue_name' => 'queue'], $httpClient) | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
$this->assertEquals( | |
new Connection(['queue_name' => 'my_queue'], new SqsClient(['region' => 'eu-west-1', 'accessKeyId' => null, 'accessKeySecret' => null], null, $httpClient)), | |
Connection::fromDsn('sqs://default/my_queue', ['queue_name' => 'queue_ignored'], $httpClient) | |
); | |
} |
f60068d
to
a6cb24d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
Good catch, thanks @ck-developer. |
Uh oh!
There was an error while loading. Please reload this page.