-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Use options from Memcached DSN #23978
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
[Cache] Use options from Memcached DSN #23978
Conversation
e85dcab
to
264d764
Compare
There are needed any work on this PR from me? |
} | ||
} | ||
|
||
public function provideDSNWithOptions() |
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.
should be provideDsnWithOptions
the provider should do a class_exists check on Memcached and yield nothing when not, in order to allow skipping the tests when Memcached is not installed
264d764
to
1363664
Compare
@nicolas-grekas fixed. |
@@ -6,6 +6,7 @@ CHANGELOG | |||
|
|||
* added PruneableInterface so PSR-6 or PSR-16 cache implementations can declare support for manual stale cache pruning | |||
* added FilesystemTrait::prune() and PhpFilesTrait::prune() implementations | |||
* added Using options from Memcached DSN |
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.
using
without capital U
1363664
to
7abc4b2
Compare
Updated |
@@ -139,11 +117,34 @@ public static function createConnection($servers, array $options = array()) | |||
if (isset($params['query'])) { | |||
parse_str($params['query'], $query); | |||
$params += $query; | |||
$options += $query; |
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 means that the precedence is:
- options provided as 2nd argument
- win over options provided as part of the DSN
- who then win over defaults.
But in RedisTrait, 1. and 2. are swapped:
- options provided as part of the DSN
- win over options provided as 2nd arg
- who then win over defaults.
This allows "the infrastructure" to take over the app config.
I think it also makes sense to do the same here.
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.
see comment about precedence
7abc4b2
to
94ad162
Compare
94ad162
to
5798dd6
Compare
@nicolas-grekas Fixed. Also updated tests for this case. |
Thank you @Bukashk0zzz. |
This PR was merged into the 3.4 branch. Discussion ---------- [Cache] Use options from Memcached DSN | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23962 | License | MIT | Doc PR | - This PR allows to pass options in cache config. Example: ```yaml framework: cache: app: cache.adapter.memcached default_memcached_provider: 'memcached://%env(MEMCACHE_HOST)%?socket_recv_size=1&socket_send_size=2' ``` Commits ------- 5798dd6 [Cache] Use options from Memcached DSN
This PR allows to pass options in cache config.
Example: