Skip to content
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

[Lock] Use platform to identify the PDO driver #43281

Merged
merged 1 commit into from Oct 4, 2021

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented Oct 1, 2021

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #43048
License MIT

This should fix the issue in the same way we did for #42011. I'm not sure if I should add/change any test...

Copy link
Member

@jderusse jderusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR revert what have been done in 7eca3a5#diff-98857ac5bb047b333495db5092ea4b875924022056c5b5522d7ecb224fac5f6f

@nicolas-grekas do you remember why you changed this in a merge commit?

Copy link
Member

@jderusse jderusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the @derrabus comment, this PR brings nothing but introduce a future deprecation for upcoming Doctrine 3.2

I'm 👎 for this change

edit: this PR fixes the issue referenced in the description

@stof
Copy link
Member

stof commented Oct 2, 2021

@jderusse this PR fixes compatibility with driver decorators (inspecting the driver class to find the target platform is a mistake). See the ticket referenced in the issue description, which describes precisely such a bug.

@jderusse
Copy link
Member

jderusse commented Oct 3, 2021

@jderusse this PR fixes compatibility with driver decorators (inspecting the driver class to find the target platform is a mistake). See the ticket referenced in the issue description, which describes precisely such a bug.

Still.. getName will be removed in the next major version (doctrine/dbal#4763) fixing the issue by introducing a dependency with a (soon) deprecated method is not the right way to fix the issue IMHO.

@chalasr
Copy link
Member

chalasr commented Oct 3, 2021

I agree, seems worth trying to fix this using instanceof checks on the platform instance instead so we don't have to revisit this too soon.

@Jean85
Copy link
Contributor Author

Jean85 commented Oct 4, 2021

I've changed the approach to use the instanceof on the platforms.

src/Symfony/Component/Lock/Store/PdoStore.php Outdated Show resolved Hide resolved
@derrabus
Copy link
Member

derrabus commented Oct 4, 2021

Thank you @Jean85.

@derrabus derrabus merged commit 72cd43a into symfony:5.4 Oct 4, 2021
@Jean85 Jean85 deleted the fix-lock-dbal-driver branch October 5, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostgreSqlStore breaks Sentry usage due to hard-coded instanceof checks.
7 participants