702271 - balrog should raise an error when data will be truncated

Status: RESOLVED WONTFIX

(Release Engineering Graveyard :: Applications: Balrog (backend), defect, P5)

Description

[[e-mail] (:bhearsum)]

bug 701865 describes a situation where we had data silently truncated by MySQL. We worked around it by using a larger field, but it's still possible for large enough data to be truncated. Balrog should throw an error when data will be truncated rather than letting it happen silently.

MySQL has a 'show warnings' command that can supposedly tell you when these sort of things happen, but we may be better off using our own method. Doing so would let us test it much better/more easily, and remain DB-agnostic.

Comment 1

[[e-mail] (:bhearsum)]

10:56 < catlee> so you may be able to configure the warnings module to raise the warning as an exception
10:56 < catlee> but you still need to call cursor.info()
10:57 < catlee> so maybe assert connection.warning_count() == 0 is ok

Comment 2

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

found in triage.

Comment 3

[[e-mail] (:bhearsum)]

We hit this again with Releases.version, bug 748399.

Comment 4

[[e-mail] (:bhearsum)]

mass component change

Comment 5

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #1)
> 10:56 < catlee> so you may be able to configure the warnings module to raise
> the warning as an exception
> 10:56 < catlee> but you still need to call cursor.info()
> 10:57 < catlee> so maybe assert connection.warning_count() == 0 is ok

Another option might be to enable strict mode, which will cause the queries to fail.

Comment 6

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #5)
> (In reply to Ben Hearsum [:bhearsum] from comment #1)
> > 10:56 < catlee> so you may be able to configure the warnings module to raise
> > the warning as an exception
> > 10:56 < catlee> but you still need to call cursor.info()
> > 10:57 < catlee> so maybe assert connection.warning_count() == 0 is ok
> 
> Another option might be to enable strict mode, which will cause the queries
> to fail.

https://github.com/mozilla/balrog/commit/d15f6cdfa23665c91021ff4b27e60a69e6b4b065 did this for the db upgrade script and it seemed to work well. We should try enabling it for the admin app and see how the errors manifest themselves. Eg, do we need to make any additional changes to get useful error messages.

Comment 7

[[e-mail] (:bhearsum)]

We haven't hit this since switching the releases.data column to LONGTEXT. I doubt this is worth fixing.