Skip to content

[DoctrineBridge] Deprecate dbal session handler #24389

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

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

Tobion
Copy link
Contributor

@Tobion Tobion commented Oct 1, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #20501
License MIT
Doc PR

The DbalSessionHandler misses all the improvements from the PdoSessionHandler (lock modes, delayed GC, configurable naming). The only advantage it had was the ability to work with non-pdo drivers. But since DBAL requires PDO now as well (doctrine/dbal@36df682) even that is not really relevant anymore.

Stofs argument in #20501 (comment) sound like a new feature that can be implemented separately. Ref. #24267

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

That's long overdue.

@nicolas-grekas
Copy link
Member

Just wondering: should we make PdoSessionHandler accept DBAL connections?

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Oct 2, 2017
@fabpot
Copy link
Member

fabpot commented Oct 5, 2017

Thank you @Tobion.

@fabpot fabpot merged commit ffc74eb into symfony:3.4 Oct 5, 2017
fabpot added a commit that referenced this pull request Oct 5, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

[DoctrineBridge] Deprecate dbal session handler

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #20501
| License       | MIT
| Doc PR        |

The DbalSessionHandler misses all the improvements from the PdoSessionHandler (lock modes, delayed GC, configurable naming). The only advantage it had was the ability to work with non-pdo drivers. But since DBAL requires PDO now as well (doctrine/dbal@36df682) even that is not really relevant anymore.

Stofs argument in #20501 (comment) sound like a new feature that can be implemented separately. Ref. #24267

Commits
-------

ffc74eb [DoctrineBridge] Deprecate DbalSessionHandler
@Tobion Tobion deleted the deprecate-dbal-session-handler branch October 5, 2017 13:02
@nicolas-grekas
Copy link
Member

PR welcomed on master to remove the legacy code.

fabpot added a commit that referenced this pull request Oct 5, 2017
This PR was merged into the 4.0-dev branch.

Discussion
----------

[Session] remove deprecated session handlers

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | yes
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | symfony/symfony-docs#8486

Removes deprecated session handler from #24389, #24438 and #24443

Commits
-------

0a7af3f [Session] remove deprecated session handlers
fabpot added a commit that referenced this pull request Oct 5, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

Add changelog for deprecated DbalSessionHandler

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Add missing changelog/upgrade for #24389

Commits
-------

af0aa1e Add changelog for deprecated DbalSessionHandler
This was referenced Oct 18, 2017
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.

5 participants