-
Notifications
You must be signed in to change notification settings - Fork 62
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
Do not use deprecated postConnect event but rely on savepoints instead #232
Conversation
0047876
to
4128968
Compare
4128968
to
2fbda57
Compare
2fbda57
to
8e3234b
Compare
77acbb8
to
3e1fc5b
Compare
{ | ||
return $this->underlyingDriver->getSchemaManager($conn, $platform); | ||
} | ||
$platform = isset($params['serverVersion']) |
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.
hi @dmaicher.
i think there is a bug here.
given config like
when@test:
doctrine:
dbal:
connections:
default:
dbname_suffix: '_test%env(default::TEST_TOKEN)%'
platform_service: App\PostgreSQLPlatform
use_savepoints: true
my custom platform service is ignored and not used here.
I thnk we need to use $params['platform']
if set, as per https://github.com/doctrine/dbal/blob/62cce0a4486ce8757fa43a3f72d95a81b89da9e7/src/Connection.php#L198
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.
Oh indeed. Looks like I missed that case. Its deprecated and won't work with DBAL 4 though.
Are you up for creating a new issue for it + look into fixing it? 😊
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, i'll fix it!
what has changed in dbal4?
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.
ah right, i assume there is a change in doctrine bundle to support this...
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.
The platform_service
option is deprecated on DoctrineBundle. I think there is no out-of-the box solution yet. Might involve creating a custom middleware + registering it.
Fixes #241