Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
e85dcab to264d764CompareBukashk0zzz commentedAug 25, 2017
There are needed any work on this PR from me? |
| } | ||
| } | ||
| publicfunctionprovideDSNWithOptions() |
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 beprovideDsnWithOptions
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 to1363664CompareBukashk0zzz commentedAug 25, 2017
@nicolas-grekas fixed. |
| * 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 to7abc4b2CompareBukashk0zzz commentedAug 27, 2017
Updated |
| 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.
nicolas-grekas left a comment
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 to94ad162Compare94ad162 to5798dd6CompareBukashk0zzz commentedSep 5, 2017
@nicolas-grekas Fixed. Also updated tests for this case. |
fabpot commentedSep 5, 2017
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:```yamlframework: 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: