Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Lock] Split PdoStore into DoctrineDbalStore#43332

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

Merged
fabpot merged 1 commit intosymfony:5.4fromGromNaN:dbal/lock-pdo-5.4
Oct 19, 2021

Conversation

@GromNaN
Copy link
Member

@GromNaNGromNaN commentedOct 6, 2021•
edited
Loading

QA
Branch?5.4
Bug fix?no
New feature?yes
Deprecations?yes
TicketsFix#42962
LicenseMIT
Doc PRWIP

Main changes:

  • New: Created classesDoctrineDbalStore andDoctrineDbalPostgreSqlStore.
  • Deprecated: UsingDoctrine\DBAL\Connection or<driver>:// DSN inPdoStore orPostgreSqlStore is deprecated in 5.4 and will be removed in 6.0. UseDoctrineDbalStore orDoctrineDbalPostgreSqlStore instead.
  • New: Mysqli driver is supported byDoctrineDbalStore (thanks to numerical params)
  • BC: When using a malformed DBAL DSN, the error will be thrown when theDoctrineDbalStore orDoctrineDbalPostgreSqlStore is instanced, instead of the first usage of the connection.

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@derrabus
Copy link
Member

I think, we can simplifyPdoStore already:

  • PdoStore gets a new private property$dbalStore.
  • IfPdoStore is constructed with a DBAL connection instance or a DBAL-compatible DSN, a deprecation is triggered and$dbalStore is initialized with aDbalStore instance.
  • In all public methods, we check if$dbalStore is not null and if that is the case, we simply proxy the call to$dbalStore.

This should allow us to remove all DBAL-related code fromPdoStore already. And on 6.0, we simply remove the proxy mechanism.

GromNaN reacted with thumbs up emoji

@derrabus
Copy link
Member

Oh, and thank you very much for working on this topic!

@GromNaNGromNaN marked this pull request as ready for reviewOctober 6, 2021 15:43
@carsonbotcarsonbot added this to the5.4 milestoneOct 6, 2021
@GromNaNGromNaNforce-pushed thedbal/lock-pdo-5.4 branch 2 times, most recently from39bcf0c to7922f8bCompareOctober 6, 2021 17:44
@GromNaNGromNaN changed the title[Lock] Split PdoStore into DbalStore[Lock] Split PdoStore into DoctrineDbalStoreOct 6, 2021
@GromNaNGromNaNforce-pushed thedbal/lock-pdo-5.4 branch 3 times, most recently from6a80a44 tof7d493aCompareOctober 7, 2021 00:03
@derrabus
Copy link
Member

Can you take a look at the failing tests?

GromNaN reacted with thumbs up emoji

Copy link
Member

@derrabusderrabus left a comment

Choose a reason for hiding this comment

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

Great work, thank you!

@fabpot
Copy link
Member

Thank you@GromNaN.

@martinssipenko
Copy link
Contributor

The newDoctrineDbalStore does not create the table automatically in case it's not present, was this done intentionally?

cc@GromNaN@derrabus

@GromNaN
Copy link
MemberAuthor

The newDoctrineDbalStore does not create the table automatically in case it's not present, was this done intentionally?

This is actually a regression.

@martinssipenko
Copy link
Contributor

@GromNaN alright, I've created an issue (#44369) and will make an attempt to fix it.

GromNaN reacted with thumbs up emoji

@GromNaNGromNaN deleted the dbal/lock-pdo-5.4 branchDecember 1, 2021 10:26
nicolas-grekas added a commit that referenced this pull requestNov 20, 2023
…able + add tests (HypeMC)This PR was merged into the 5.4 branch.Discussion----------[Cache][HttpFoundation][Lock] Fix PDO store not creating table + add tests| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITUnlike e.g. SQLite, PostgreSQL doesn't throw an exception on `$conn->prepare()` if the table is missing, it instead throws it on `$stmt->execute()`, so the table never gets created.The table used to get created, but the behavior was broken with#43332.Commits-------cb5d832 [Cache][Lock] Fix PDO store not creating table + add tests
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@derrabusderrabusderrabus approved these changes

@jderussejderusseAwaiting requested review from jderussejderusse is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

Split PDO and DBAL adapters

7 participants

@GromNaN@carsonbot@derrabus@fabpot@martinssipenko@stof@nicolas-grekas
[8]ページ先頭
©2009-2025 Movatter.jp