sentry-symfony icon indicating copy to clipboard operation
sentry-symfony copied to clipboard

Deprecation warning with doctrine/dbal 3.2.0

Open Offout opened this issue 2 years ago • 14 comments

Environment

sentry/sentry-symfony 4.2.5 doctrine/dbal 3.2.0 Symfony - 5.3, 5.4

Steps to Reproduce

Run any code, look for deprecations warnings

Expected Result

No deprecation warning

Actual Result

The "Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverForV3" class implements "Doctrine\DBAL\VersionAwarePlatformDriver" that is deprecated All drivers will have to be aware of the server version in the next major release.

Why it's happening

https://github.com/doctrine/dbal/pull/4751

Offout avatar Dec 13 '21 16:12 Offout

I add the following warning to the issue.

The "Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingServerInfoAwareDriverConnection" class implements "Doctrine\DBAL\Driver\ServerInfoAwareConnection" that is deprecated The methods defined in this interface will be made part of the {@link Driver} interface in the next major release.

duboiss avatar Dec 17 '21 12:12 duboiss

Please see the referenced issue to understand why it is not possible to solve these deprecations and what's the best solution for the moment

ste93cry avatar Dec 17 '21 15:12 ste93cry

Also:

Deprecated: PDOStatement::bindParam(): Passing null to parameter #4 ($maxLength) of type int is deprecated in Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingStatementForV3::bindParam at line 30

Jeroeny avatar Dec 20 '21 11:12 Jeroeny

I checked the interface for version 3.2 of the DBAL and the $length argument still accepts int|null, so I don't think that the problem is us, but rather who is calling the code :thinking:

ste93cry avatar Dec 20 '21 12:12 ste93cry

Indeed, It looks like it might actually be Doctrine calling PDO that causes that one. So unrelated to this issue and repo after all.

Edit: looks like Doctrine passes on the $length parameter if given. Since Sentry always passed $length, even when null, the deprecation can be considered as caused by Sentry. I've made a PR: https://github.com/getsentry/sentry-symfony/pull/586

Jeroeny avatar Dec 20 '21 13:12 Jeroeny

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

github-actions[bot] avatar Jan 13 '22 12:01 github-actions[bot]

Could someone please add the label Status: In Progress to prevent this issue from being closed :-)

mwansinck avatar Jan 18 '22 11:01 mwansinck

Can someone of Sentry give an update on this issue?

The "Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverForV3" class implements "Doctrine\DBAL\VersionAwarePlatformDriver" that is deprecated All drivers will have to be aware of the server version in the next major release.

I don't like warnings like this during my test flow's... and even worse, in a next release it will break things! Please make an appropriate fix for it.

weerdenburg avatar Mar 06 '22 21:03 weerdenburg

Note that implementing that interface results in calls to connect() when calling getDatabasePlatform even if the underlying driver is not versionaware (like sqlite).

greg0ire avatar Mar 25 '22 16:03 greg0ire

Hi all,

Is there anything new about this deprecation warning?

Cheers

LaurentSanson avatar Jan 06 '23 13:01 LaurentSanson

I created a PR to address this problem. Please have a look:

  • https://github.com/getsentry/sentry-symfony/pull/723

ruudk avatar Jun 06 '23 17:06 ruudk

Copying here from #734 my comment as per @cleptric's request

I tried looking at the changes that have been made in the DBAL in the next future major version (4.0) and as far as I see, the way this deprecation has been handled is a real mess:

  • The VersionAwarePlatformDriver interface was marked as deprecated in doctrine/dbal#4751. However, the check that ensures that the interface gets implemented was left unaltered, giving no way to avoid triggering the deprecation. I would have expected it to change to something that also takes into account the existence of the method itself 🤔

  • The VersionAwarePlatformDriver interface has been removed in doctrine/dbal#4764. However, the original method createDatabasePlatformForVersion() has been removed as well, altering the existing getDatabasePlatform() method instead and making it version-aware.

To me, there is something missing for a deprecation-free and clean upgrade path from version 3.x to version 4.0 of the DBAL. But, there is also another thing to say: this deprecation is actually not triggered by the Doctrine package; as I said for similar other deprecations, this is PHPUnit Bridge not minding its own business and triggering notices all around as soon as it sees a @deprecation tag. Maybe we should revert all the changes and just tell everyone to suppress this deprecation using the baseline.

@morozov since you are the original author of the changes in the DBAL, can you give us some hints on how you envisioned the upgrade path?

ste93cry avatar Jun 19 '23 13:06 ste93cry

Maybe we should revert all the changes and just tell everyone to suppress this deprecation using the baseline.

So the solution here is to ignore it? I'm confused 😕

craigh avatar Dec 08 '23 18:12 craigh

Yesterday doctrine/orm 3 (using doctrine/dbal 4) was released. so now this deprecation is a fatal error 😢

garak avatar Feb 04 '24 11:02 garak