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

feat: fix dbal deprecation for connection #268

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

Chris53897
Copy link
Contributor

#259

This fix does makes it possible, to use it with my bundle and orm3 dbal4
I got a NotSupported Exception with SQLite-Database.

Please have a look if changes are correct.

@alexislefebvre
Copy link
Collaborator

Could you please explain the try … catch block?

If the goal is to call createSchemaManager when it's available and fallback to getSchemaManager, I suggest to use a BC approach:

if (method_exists($this->connection, 'fetchColumn')) {
$currentValue = $this->connection->fetchColumn('SELECT @@SESSION.foreign_key_checks');
} else {
$currentValue = $this->connection->fetchOne('SELECT @@SESSION.foreign_key_checks');
}

@Chris53897
Copy link
Contributor Author

If i run it in my bundle for testing. I got a NotSupported Exception with SQLite-Database.
The try … catch block solves this.

@mbabker Can you please check if my changes are correct?

@alexislefebvre
Copy link
Collaborator

Thanks for the explanation.

I suggest to add a rule to ignore this call when the connection is a SQLite one. So it won't need a try … catch block.

@mbabker
Copy link
Contributor

mbabker commented Feb 25, 2024

I suggest to add a rule to ignore this call when the connection is a SQLite one. So it won't need a try … catch block.

Based on https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#bc-break-removed-sqliteschemamanagercreatedatabase-and-dropdatabase-methods I'd say the try/catch might actually be more appropriate while DBAL 3.x is supported. As the functionality is only removed from 4.0, it avoids getting into deeper instanceof checks plus some other check to work out the active DBAL version. Whereas the instanceof check would immediately turn this feature off for all SQLite users no matter the DBAL version.

@alexislefebvre alexislefebvre merged commit a75285a into liip:2.x Feb 25, 2024
11 checks passed
@alexislefebvre
Copy link
Collaborator

Thanks you all for the PR and comments, here is a new release with these changes: https://github.com/liip/LiipTestFixturesBundle/releases/tag/2.7.3

@Chris53897 Chris53897 deleted the feature/fix-dbal-deprecation branch February 26, 2024 07:06
@Chris53897
Copy link
Contributor Author

thank you @alexislefebvre @mbabker

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.

None yet

4 participants