-
Notifications
You must be signed in to change notification settings - Fork 439
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
[sns] added possibility to define already existing topics (prevent create topic call) #1022 #1147
Conversation
pkg/sns/SnsContext.php
Outdated
{ | ||
$this->client = $client; | ||
$this->config = $config; | ||
|
||
$this->topicArns = []; | ||
$this->topicArns = $topicArns; |
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.
topicArns property format is
topic_name => arn
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.
yep, refactored
pkg/sns/SnsConnectionFactory.php
Outdated
@@ -71,7 +72,7 @@ public function __construct($config = 'sns:') | |||
*/ | |||
public function createContext(): Context | |||
{ | |||
return new SnsContext($this->establishConnection(), $this->config); | |||
return new SnsContext($this->establishConnection(), $this->config, $this->config['topic_arns']); |
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.
why do you pass topc_arns as separate arg? They are already there in config which is passed as second arg
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.
removed third arg
pkg/sns/SnsConnectionFactory.php
Outdated
@@ -59,6 +60,9 @@ public function __construct($config = 'sns:') | |||
|
|||
unset($config['dsn']); | |||
} | |||
if (\array_key_exists('topic_arns', $config) && \is_string($config['topic_arns'])) { |
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.
at this stage it has to be assoc array already. The string parsing should happen if dsn is passed.
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.
removed, not needed when using dsn parsing
]; | ||
|
||
yield [ | ||
['dsn' => 'sns:?topic_arns=topic1|arn:aws:sns:us-east-1:123456789012:topic1;topic2|arn:aws:sns:us-west-2:123456789012:topic2'], |
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.
what about passing arns as
topic_arns[topic1]=arn:aws:sns:us-east-1:123456789012&topic_arns[topic2]=arn:aws:sns:us-west-2:123456789012:topic2
that way the standard query parsing function does the job
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.
good point, replaced
fixes #1022 |
No description provided.