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

Fix master-slaves deprecation from dbal 2.11 #1215

Merged
merged 1 commit into from
Oct 3, 2020

Conversation

Dodenis
Copy link
Contributor

@Dodenis Dodenis commented Sep 25, 2020

Since dbal 2.11 MasterSlaveConnection is deprecated and the doctrine:database:create and doctrine:database:drop commands no longer work with the "slaves" option. doctrine/dbal#4054

Issue : #1214

@ostrolucky
Copy link
Member

You should rather report a BC break in DBAL repo.

@stof
Copy link
Member

stof commented Sep 25, 2020

I'm quite sure this PR makes a BC break in the bundle config by removing support for slaves keys as soon as the new version of DBAL is installed.

What we need is a proper migration to the new names, where the bundle config also uses them and deprecates master/slave. Then, the bundle would use the right name on the DBAL side based on the DBAL version (whatever the user-facing name is).

My proposal is to fix only the commands in that PR (to fix compat with 2.11), and perform the master/slave to primary/replicas in the config in a separate PR.

@ostrolucky no. The 2 commands are using Connection::getParams which is tagged as @internal so that cannot be considered as a regression in DBAL.

@Dodenis
Copy link
Contributor Author

Dodenis commented Sep 25, 2020

@stof I'm agree with you.

I modify this PR to fix only the commands

@kralos
Copy link

kralos commented Sep 25, 2020

You should rather report a BC break in DBAL repo.

See doctrine/dbal#4295

@greg0ire greg0ire requested a review from ostrolucky September 28, 2020 08:55
@ostrolucky
Copy link
Member

Let's merge after doctrine/dbal#4308 is merged. If we merge this first, there will no longer be a pressure to fix a BC break in DBAL. This is not a pressing issue, people can just hold off from upgrading dbal.

@ostrolucky ostrolucky added Status: On Hold Most likely waiting for upstream resolution and removed Status: On Hold Most likely waiting for upstream resolution labels Sep 28, 2020
@ostrolucky ostrolucky added this to the 1.12.11 milestone Oct 3, 2020
@ostrolucky ostrolucky merged commit c294d32 into doctrine:1.12.x Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants