Open Stack

On Fri, Jan 24, 2014 at 3:29 AM, Florian Haas <florian at hastexo.com> wrote:

> On Thu, Jan 23, 2014 at 7:22 PM, Ben Nemec <openstack at nemebean.com> wrote:
> > On 2014-01-23 12:03, Florian Haas wrote:
> >
> > Ben,
> >
> > thanks for taking this to the list. Apologies for my brevity and for
> HTML,
> > I'm on a moving train and Android Gmail is kinda stupid. :)
> >
> > I have some experience with the quirks of phone GMail myself. :-)
> >
> > On Jan 23, 2014 6:46 PM, "Ben Nemec" <openstack at nemebean.com> wrote:
> >>
> >> A while back a change (https://review.openstack.org/#/c/47820/) was
> made
> >> to allow enabling mysql traditional mode, which tightens up mysql's
> input
> >> checking to disallow things like silent truncation of strings that
> exceed
> >> the column's allowed length and invalid dates (as I understand it).
> >>
> >> IMHO, some compelling arguments were made that we should always be using
> >> traditional mode and as such we started logging a warning if it was not
> >> enabled.  It has recently come to my attention
> >> (https://review.openstack.org/#/c/68474/) that not everyone agrees, so
> I
> >> wanted to bring it to the list to get as wide an audience for the
> discussion
> >> as possible and hopefully come to a consensus so we don't end up having
> this
> >> discussion every few months.
> >
> > For the record, I obviously am all in favor of avoiding data corruption,
> > although it seems not everyone agrees that TRADITIONAL is necessarily the
> > preferable mode. But that aside, if Oslo decides that any particular
> mode is
> > required, it should just go ahead and set it, rather than log a warning
> that
> > the user can't possibly fix.
> >
> >
> > Honestly, defaulting it to enabled was my preference in the first place.
>  I
> > got significant pushback though because it might break consuming
> > applications that do the bad things traditional mode prevents.
>
> Wait. So the reasoning behind the pushback was that an INSERT that
> shreds data is better than an INSERT that fails? Really?
>
> > My theory
> > was that we could default it to off, log the warning, get all the
> projects
> > to enable it as they can, and then flip the default to enabled.
>  Obviously
> > that hasn't all happened though. :-)
>
> Wouldn't you think it's a much better approach to enable whatever mode
> is deemed appropriate, and have malformed INSERTs (rightfully) break?
> Isn't that a much stronger incentive to actually fix broken code?
>
> The oslo tests do include a unit test for this, jftr, checking for an
> error to be raised when a 512-byte string is inserted into a 255-byte
> column.
>
> > Hence my proposal to make this a config option. To make the patch as
> > un-invasive as possible, the default for that option is currently empty,
> but
> > if it seems prudent to set TRADITIONAL or STRICT_ALL_TABLES instead,
> I'll be
> > happy to fix the patch up accordingly.
> >
> > Also check out Jay's reply.  It sounds like there are some improvements
> we
> > can make as far as not logging the message when the user enables
> traditional
> > mode globally.
>
> And then when INSERTs break, it will be much more difficult for an
> application developer to figure out the problem, because the breakage
> would happen based on a configuration setting outside the codebase,
> and hence beyond the developer's control. I really don't like that
> idea. All this leads to is bugs being filed and then closed with a
> simple "can't reproduce."
>
> > I'm still not clear on whether there is a need for the STRICT_* modes,
> and
> > if there is we should probably also allow STRICT_TRANS_TABLES since that
> > appears to be part of "strict mode" in MySQL.  In fact, if we're going to
> > allow arbitrary modes, we may need a more flexible config option - it
> looks
> > like there are a bunch of possible sql_modes available for people who
> don't
> > want the blanket "disallow all the things" mode.
>
> Fair enough, I can remove the "choices" arg for the StrOpt, if that's
> what you suggest. My concern was about unsanitized user input. Your
> inline comment on my patch seems to indicate that we should instead
> trust sqla to do input sanitization properly.
>
> I still maintain that leaving $insert_mode_here mode off and logging a
> warning is silly. If it's necessary, turn it on and have borked
> INSERTs fail. If I understand the situation correctly, they would fail
> anyway the moment someone switches to, say, Postgres.
>

+1

Doug



>
> Cheers,
> Florian
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140124/d909e6b0/attachment.html>