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] Add PDO + Doctrine DBAL adapter#19519
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
bb2dba9 to6a46b9eCompare| private function init($pdoOrDsn, $namespace, $defaultLifetime, array $options) | ||
| { | ||
| if (isset($namespace[0]) && preg_match('#[^-+.A-Za-z0-9]#', $namespace, $match)) { |
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.
Wouldn'tstrpbrk be more efficient ?
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.
not sure, less readable, and not perf critical code path anyway (run once code) :)
ab167dc todcc7915Compare| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| protected $maxIdLength = 255; |
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.
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.
phpdoc seems useless to me
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
nicolas-grekas commentedAug 6, 2016
This now also accepts a Doctrine DBAL Connection. |
| "require-dev": { | ||
| "cache/integration-tests":"dev-master", | ||
| "doctrine/cache":"~1.6", | ||
| "doctrine/dbal":"~2.4", |
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.
From a "component" point of view, you may adddoctrine/dbal to thesuggest section, what do you think ?
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.
Bof... This would never end: install redis, install apcu, install... etc. No value. Read the doc instead (I'm on it).
11e8765 to5007996Compare| $value =false; | ||
| }elseif (false ===$value =unserialize($value)) { | ||
| $value =null; | ||
| $this->values[$key] =$value =null; |
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.
isn't this a bugfix which should go separately ?
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#19640
| break; | ||
| case 'pdo_sqlsrv': | ||
| $this->driver = 'sqlsrv'; | ||
| break; |
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 handling the default case here, for unknown drivers ?
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.
we use only pgsql and sqlsrv, so I'd rather remove all the unused cases than add a default
nicolas-grekasAug 16, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
ok, bad answer :) yet, the adapter doesn't handle other drivers, so we don't care for a default case here
fabpot commentedAug 19, 2016
Thank you@nicolas-grekas. |
This PR was merged into the 3.2-dev branch.Discussion----------[Cache] Add PDO + Doctrine DBAL adapter| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR |symfony/symfony-docs#6858This PR adds a PDO adapter for our PSR-6 cache items. The implementation heavily borrows from PdoSessionHandler.Commits-------82a0de2 [Cache] Add PDO + Doctrine DBAL adapter
Uh oh!
There was an error while loading.Please reload this page.
This PR adds a PDO adapter for our PSR-6 cache items. The implementation heavily borrows from PdoSessionHandler.