[Lock][Messenger] Fix precedence of DSN options for 5.1 #37269
Conversation
jderusse
force-pushed
the
fix-precedence-51
branch
from
f9b41c5
to
85fff57
Compare
|
There are some failures to take care of. |
jderusse
force-pushed
the
fix-precedence-51
branch
2 times, most recently
from
b409d15
to
c4942b8
Compare
| @@ -120,7 +120,7 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300 | |||
| if (isset($parsedUrl['query'])) { | |||
| parse_str($parsedUrl['query'], $query); | |||
| } | |||
| $this->options['collection'] = $this->options['collection'] ?? $query['collection'] ?? null; | |||
| $this->options['collection'] = $query['collection'] ?? $this->options['collection'] ?? null; | |||
There was a problem hiding this comment.
Isn't this a BC Break? someone's application could start writing to the wrong collection if they are specifying both for some reason and they differ. Also there's the path in the uri which should take precedence over $options['database']
There was a problem hiding this comment.
I would rather we deal with Lock\Store\MongoDbStore in #37218
There was a problem hiding this comment.
see #37218 (comment) about BC vs bugfix
There was a problem hiding this comment.
there's the path in the uri which should take precedence over $options['database']
@jderusse can you have a look at this, also in the other PR?
There was a problem hiding this comment.
good catch. Also fix precedenceof credentials in that PR
There was a problem hiding this comment.
did not found precendence in #37268 @nicolas-grekas . Did you have a specific thing in mind?
jderusse
force-pushed
the
fix-precedence-51
branch
from
3908741
to
132b5ea
Compare
nicolas-grekas
force-pushed
the
fix-precedence-51
branch
from
132b5ea
to
9670e9f
Compare
|
Thank you @jderusse. |
… (jderusse) This PR was merged into the 5.1 branch. Discussion ---------- [Messenger] Fix invalid option sslmode in AmazonSqs bridge | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | /> | License | MIT | Doc PR | / The sslmode option allows people to use AmazonSqs with non-offical endpoint like the [asyncaws/testing-sqs](https://hub.docker.com/r/asyncaws/testing-sqs) docker image By fixing precedence of DNS options in #37269 I introduced a bug that trigger an exception `Unknown option found: [sslmode]`. I apologize for this This PR adds `sslmode` in list of allowed options Commits ------- afbd51b Fix invalid option sslmode
This PR fix précédence of DSN options over constructor options in all component on branch 5.1