Request for removal: Mysql warnings get promoted to Exceptions in debug mode

[Ilja Maas]

Investigating https://code.djangoproject.com/ticket/21163
I came across these lines in mysql/base.py:

if settings.DEBUG:
    warnings.filterwarnings("error", category=Database.Warning)


Finding out why, this bubbles up to somewhere in 2007
https://github.com/django/django/commit/f9c4ce51235aac4862cfe2dfaaf6836acaea1c3d


I can't find the reason why this behaviour is in, other than the comments in de commit mentioned before: "Since the point is to raise Warnings as exception"..

No arguments here..  Is there someone who actually knows the reasons?
It seems this code comes from the old MySQL days (pre 5/5.1) and a lot has changed since in MySQL since then.

However.. I think in DEBUG mode these warning should be shown as warnings as they are warnings :-) an not being promoted to Exceptions.
Maybe even _every_ warning should be shown and not only the first one. The example in the bug-report is about data-truncation.
It's not something like a deprecation warning.

Coming to a conclusion _IMHO_ the different behaviour between debug mode or not is somehow strange.  

The behaviour should be either promote warning to exceptions in DEBUG=False (production) too, but that probably breaks a lot of django implementations.

or, get the promotion out.

Any suggestions/ideas on this?


PS.
With these lines removed, none of the unittests fail.

[Chi Ho Kwok]

This is working as intended; on most databases, inserting a 11 character text value into a VARCHAR(10) raises an error. On MySQL, depending on if strict mode is enabled*, it either inserts "just fine" by truncating "12345678901" to "1234567890" with a warning, or fails with an error.

As silent data corruption isn't a good thing Django currently turns the warning into an error, at least if DEBUG is set. Not all warnings are this evil so doing this in production is a no go, but the one in https://code.djangoproject.com/ticket/21163 is a clear case of programming error, inserting 11 chars into a max_length=10 field should fail without doubt, it's silly that MySQL accepts it at all.

As for the unit test, in my opinion, there should be a test case for MySQL that asserts an error is thrown with DEBUG enabled and forcing a VARCHAR field overflow. In reality, it works now so no-one will touch this.

* non strict is the default prior to MySQL 5.7.5. Strict mode in transactions is default for 5.7.5 and newer. Strict mode is off by default outside of transactions.

[Karen Tracey]

Hmm, looking at the code that existed before the filterwarnings promotion to error, the older code was catching Database.Warning exceptions and re-raising them with the warning information included. This was before my time but it seems the older code was simply making the exceptions raised for DB warnings more informative in DEBUG mode, by fetching and including the warning text in the exception?

I agree it seems odd to have warnings turned into exceptions for DEBUG=True only. While it is longstanding behavior it doesn't make much sense nowadays when Django has better logging support (so developers can hopefully "see" the warnings if just logged as such) and MySQL has moved more towards having strict mode be the default.

I think it would make sense to get rid of this behavior and just log MySQL warnings as warnings.

Karen

[Anssi Kääriäinen]

I don't see any reason why we should promote warnings to errors only
when DEBUG is enabled.

+1 to removing the filterwarnings() call.

- Anssi

[Florian Apolloner]

Imo we should make those exceptions instead! Data truncation should never be just a warning ;)

Cheers,
Florian

[Aymeric Augustin]

2014-11-17 10:45 GMT+01:00 Florian Apolloner <f.apo...@gmail.com>:

Imo we should make those exceptions instead! Data truncation should never be just a warning ;)

Those who think like you can use a database instead of MySQL ;-)

I agree we should simply remove the code that promotes warnings to exceptions in DEBUG mode because :

- there's absolutely no reason to be more strict in development than in production

- promoting warnings to exceptions in production would be subtly backwards-incompatible

- you can enable this behavior in the MySQL configuration e.g. by enabling Strict SQL Mode

 

--

Aymeric.

[Florian Apolloner]

On Monday, November 17, 2014 11:40:15 AM UTC+1, Aymeric Augustin wrote:

2014-11-17 10:45 GMT+01:00 Florian Apolloner <f.apo...@gmail.com>:

Imo we should make those exceptions instead! Data truncation should never be just a warning ;)

Those who think like you can use a database instead of MySQL ;-)

Touché :)

[Tim Graham]

Just to close the loop, I created a ticket and will commit shortly: https://code.djangoproject.com/ticket/23871

[Ilja Maas]

Great!  Thanx

Op zaterdag 15 november 2014 18:08:34 UTC+1 schreef Ilja Maas: