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

Deprecate the Synchronizer package #4213

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Aug 18, 2020

Q A
Type deprecation
BC Break no

Summary

The SchemaSynchronizer API was introduced as part of the support for sharding (#162) which is now deprecated (#3905). This code hasn't received any functional changes in the last 8 years (12f381f) and doesn't seem to be used by any open source projects.

According to the tests and the implementation, SingleDatabaseSynchronizer is just a wrapper over other schema-related APIs and doesn't provide any additional functionality.

@morozov
Copy link
Member Author

morozov commented Aug 18, 2020

From #162:

It is self-contained and does not affect the normal DBAL code.

It's a perfect example of why “let's add this code to the library because another project may need to use it” is a bad idea.

@greg0ire
Copy link
Member

There still seems to be references to this in docs: https://github.com/doctrine/dbal/blob/2.11.x/docs/en/reference/sharding.rst#schema-operations-schemasynchronizer-interface Should these docs have been removed, or maybe marked as deprecated in #3905 ? IMO we should not advertise deprecated features.

@morozov
Copy link
Member Author

morozov commented Aug 18, 2020

IMO we should not advertise deprecated features.

Well, we're not necessarily advertising them, it's just documentation. I'll make sure it's removed during the up-merge. In any event, the whole "Sharding" section will be removed from the documentation, not only this specific API.

@greg0ire
Copy link
Member

I mean that we should maybe remove it from 2.11.x too, or at least make it clear that sharding is deprecated in the docs

greg0ire
greg0ire previously approved these changes Aug 18, 2020
@morozov
Copy link
Member Author

morozov commented Aug 18, 2020

I mean that we should maybe remove it from 2.11.x too […]

Removing documentation for the existing code looks odd to me.

[…] or at least make it clear that sharding is deprecated in the docs

We haven't been doing this for any other deprecations, so we're at least consistent here. Otherwise, there's a lot more to update than just this.

UPD: in fact, what the right approach is depends on what type of documentation that is. It would indeed make sense to remove it from a How-To or a Tutorial but kept in Explanation or Reference. Ours is all-in-once, so there's no right or wrong answer.

@morozov morozov added this to the 2.11.0 milestone Aug 18, 2020
@greg0ire
Copy link
Member

Nice resource, thanks for sharing it!
After a quick look, I'd say that https://github.com/doctrine/dbal/blob/2.10.x/docs/en/reference/sharding.rst is more of a How-to guide, while
https://github.com/doctrine/dbal/blob/2.10.x/docs/design/SHARDING.md looks like an explanation, but there might be other types leaking in both documents. What would you say the right approach is for each type?

@morozov
Copy link
Member Author

morozov commented Aug 19, 2020

What would you say the right approach is for each type?

I suggest we take the same approach as for sharding: don't make any documentation changes during the deprecation (#3905) and removed the documentation during the code removal (#3906).

If we want to change this process, it's out of the scope of this issue and should be done separately.

@morozov
Copy link
Member Author

morozov commented Aug 19, 2020

On the other hand, we do have some APIs (e.g. platforms) marked as deprecated in the documentation. But it's more of an exception than a rule:

$ git grep -i deprecated lib/ 2.11.x | wc -l
354

$ git grep -i deprecated docs/ 2.11.x | wc -l
15

I went ahead and updated the documentation on SchemaSynchronizer.

@morozov morozov merged commit 92419f0 into doctrine:2.11.x Aug 19, 2020
@morozov morozov deleted the deprecate-schema-synchronizer branch August 19, 2020 17:30
@morozov morozov self-assigned this Aug 19, 2020
fabpot added a commit to symfony/symfony that referenced this pull request Aug 21, 2020
…API (xabbuh)

This PR was merged into the 4.4 branch.

Discussion
----------

[Messenger] stop using the deprecated schema synchronizer API

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

see the failing tests and doctrine/dbal#4213

Commits
-------

40129d6 stop using the deprecated schema synchronizer API
beberlei added a commit to beberlei/dbal that referenced this pull request Mar 6, 2021
beberlei added a commit to beberlei/dbal that referenced this pull request Mar 7, 2021
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Mar 7, 2021
Release [2.11.0](https://github.com/doctrine/dbal/milestone/77)

2.11.0
======

- Total issues resolved: **7**
- Total pull requests resolved: **55**
- Total contributors: **8**

Improvement,Prepared Statements,SQL Server,Types,pdo_sqlsrv,sqlsrv
------------------------------------------------------------------

 - [4274: Support ASCII parameter binding](doctrine#4274) thanks to @gjdanis

Documentation
-------------

 - [4271: Add explanation about implicit indexes](doctrine#4271) thanks to @greg0ire

Deprecation,Error Handling
--------------------------

 - [4253: Deprecate DBAL\DBALException in favor of DBAL\Exception](doctrine#4253) thanks to @morozov

CI
--

 - [4251: Setup automatic release workflow](doctrine#4251) thanks to @greg0ire

Deprecation,Schema Managers
---------------------------

 - [4230: Deprecate the functionality of dropping client connections when dropping a database](doctrine#4230) thanks to @morozov

Deprecation,Platforms
---------------------

 - [4229: Deprecate more AbstractPlatform methods](doctrine#4229) thanks to @morozov
 - [4132: Deprecate AbstractPlatform::fixSchemaElementName()](doctrine#4132) thanks to @morozov

Improvement,Test Suite
----------------------

 - [4215: Remove test group configuration leftovers](doctrine#4215) thanks to @morozov
 - [4080: Update PHPUnit to 9.2](doctrine#4080) thanks to @morozov
 - [4079: Forward compatibility with PHPUnit 9.3](doctrine#4079) thanks to @morozov
 - [3923: Removed performance tests](doctrine#3923) thanks to @morozov

Deprecation,Schema
------------------

 - [4213: Deprecate the Synchronizer package](doctrine#4213) thanks to @morozov

Blocker,Improvement,PHP,Test Suite
----------------------------------

 - [4201: Update PHPUnit to 9.3](doctrine#4201) thanks to @morozov

Blocker,PHP,Test Suite
----------------------

 - [4196: The test suite uses the deprecated at() matcher](doctrine#4196) thanks to @morozov

Connections,Deprecation,Documentation
-------------------------------------

 - [4175: Additional deprecation note for PrimaryReplicaConnection::query()](doctrine#4175) thanks to @morozov

Connections,Deprecation,Prepared Statements
-------------------------------------------

 - [4165: Deprecated usage of wrapper components as implementations of driver-level interfaces](doctrine#4165) thanks to @morozov
 - [4020: Deprecated Connection::project(), Statement::errorCode() and errorInfo()](doctrine#4020) thanks to @morozov

Connections,Deprecation
-----------------------

 - [4163: Deprecate duplicate and ambiguous wrapper connection methods](doctrine#4163) thanks to @morozov

Error Handling,Improvement,Types
--------------------------------

 - [4145: Add TypeRegistry constructor](doctrine#4145) thanks to @morozov

Deprecation,Drivers,Improvement,pdo_mysql,pdo_oci,pdo_pgsql,pdo_sqlite,pdo_sqlsrv
---------------------------------------------------------------------------------

 - [4144: Deprecate classes in Driver\PDO* namespaces](doctrine#4144) thanks to @morozov

Connections,Documentation,Improvement
-------------------------------------

 - [4139: Mark connection constructors internal](doctrine#4139) thanks to @morozov

Deprecation,Drivers,Error Handling
----------------------------------

 - [4137: Deprecate driver exception conversion APIs](doctrine#4137) thanks to @morozov
 - [4112: Deprecate DriverException::getErrorCode()](doctrine#4112) thanks to @morozov

Configuration,Connections,Deprecation,Error Handling
----------------------------------------------------

 - [4134: Deprecate some DBALException factory methods](doctrine#4134) thanks to @morozov

Code Style,Documentation
------------------------

 - [4133: Fix more issues introduced by the deprecation of driver classes](doctrine#4133) thanks to @morozov

BC Break,Drivers,Error Handling,pdo_sqlsrv,sqlsrv
-------------------------------------------------

 - [4131: Restore the PortWithoutHost exception parent class](doctrine#4131) thanks to @morozov

Code Style,Improvement,Static Analysis
--------------------------------------

 - [4123: Remove the no longer needed error suppressions](doctrine#4123) thanks to @morozov

Deprecation,Drivers,Error Handling,Improvement
----------------------------------------------

 - [4118: Deprecate ExceptionConverterDriver](doctrine#4118) thanks to @morozov

Bug,Connections,Improvement,Prepared Statements
-----------------------------------------------

 - [4117: Fixes for the recently introduced driver-level deprecations](doctrine#4117) thanks to @morozov

Connections,Deprecation,Platform Detection
------------------------------------------

 - [4114: Deprecate ServerInfoAwareConnection#requiresQueryForServerVersion() as an implementation detail](doctrine#4114) thanks to @morozov

Deprecation,Prepared Statements,SQL Parser,oci8
-----------------------------------------------

 - [4110: Mark non-interface OCI8 driver methods internal](doctrine#4110) thanks to @morozov

Connections,Deprecation,Drivers,Improvement,Prepared Statements
---------------------------------------------------------------

 - [4100: Deprecate inconsistently and ambiguously named driver-level classes](doctrine#4100) thanks to @morozov

Connections,Improvement
-----------------------

 - [4092: Remove Connection::$isConnected](doctrine#4092) thanks to @morozov

Configuration,Connections
-------------------------

 - [4086: Mark Connection::getParams() internal](doctrine#4086) thanks to @morozov

Bug,Drivers,ibm_db2
-------------------

 - [4085: The IBM DB2 driver Exception class must implement the DriverException interface](doctrine#4085) thanks to @morozov

PHP
---

 - [4078: Bump PHP requirement to 7.3 as of DBAL 2.11.0](doctrine#4078) thanks to @morozov

Connections,Databases,Deprecation,Drivers
-----------------------------------------

 - [4068: Deprecate Driver::getDatabase()](doctrine#4068) thanks to @morozov

Deprecation,Improvement,Portability
-----------------------------------

 - [4061: Deprecated platform-specific portability mode constants](doctrine#4061) thanks to @morozov

 - [4054: &doctrine#91;doctrineGH-4052&doctrine#93; Deprecate MasterSlaveConnection and rename to PrimaryReplicaConnection](doctrine#4054) thanks to @beberlei and @albe
 - [4017: Improve help of dbal:run-sql command](doctrine#4017) thanks to @ostrolucky

Code Style,Improvement
----------------------

 - [4050: Update doctrine/coding-standard to 8.0](doctrine#4050) thanks to @morozov

Cache,Deprecation,Improvement,Prepared Statements
-------------------------------------------------

 - [4049: Replace forward-compatible ResultStatement interfaces with Result](doctrine#4049) thanks to @morozov

Cache,Improvement,Prepared Statements
-------------------------------------

 - [4048: Make caching layer not rely on closeCursor()](doctrine#4048) thanks to @morozov

Deprecation,Improvement,Prepared Statements
-------------------------------------------

 - [4037: Introduce Statement::fetchFirstColumn()](doctrine#4037) thanks to @morozov
 - [4019: Deprecated the concept of the fetch mode](doctrine#4019) thanks to @morozov

Bug,Documentation,Improvement,Prepared Statements
-------------------------------------------------

 - [4034: Additional changes based on the discussion in doctrine#4007](doctrine#4034) thanks to @morozov

Connections,Console,Improvement
-------------------------------

 - [3956: allow using multiple connections for CLI commands](doctrine#3956) thanks to @dmaicher

Deprecation,Logging
-------------------

 - [3935: Deprecate EchoSQLLogger](doctrine#3935) thanks to @morozov

Improvement,Packaging
---------------------

 - [3924: Actualize the content of the .gitattributes file](doctrine#3924) thanks to @morozov

Azure,Deprecation,Drivers,Drizzle,MariaDB,Platforms,PostgreSQL,SQL Anywhere,SQL Server,Sharding,pdo_ibm
-------------------------------------------------------------------------------------------------------

 - [3905: Deprecate the usage of the legacy platforms and drivers](doctrine#3905) thanks to @morozov

Deprecation,Query
-----------------

 - [3864: CompositeExpression and()/or() factory methods](doctrine#3864) thanks to @BenMorel
 - [3853: Deprecate calling QueryBuilder methods with an array argument](doctrine#3853) thanks to @BenMorel and @morozov

Deprecation,Tools
-----------------

 - [3861: Deprecated the usage of the Version class](doctrine#3861) thanks to @morozov

Improvement,Query
-----------------

 - [3852: First parameter of ExpressionBuilder::and/or() mandatory](doctrine#3852) thanks to @BenMorel

Deprecation,Improvement,Query
-----------------------------

 - [3851: Rename andX() / orX() methods](doctrine#3851) thanks to @BenMorel
 - [3850: Prepare CompositeExpression for immutability](doctrine#3850) thanks to @BenMorel

# gpg: Signature made Mon Sep 21 01:47:31 2020
# gpg:                using DSA key 1BEDEE0A820BC30D858F9F0C2C3A645671828132
# gpg: Can't check signature: No public key

# Conflicts:
#	README.md
#	lib/Doctrine/DBAL/Driver/AbstractOracleDriver.php
#	lib/Doctrine/DBAL/Driver/OCI8/Driver.php
#	lib/Doctrine/DBAL/Version.php
beberlei added a commit to beberlei/dbal that referenced this pull request Mar 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants