On Fri, Jan 24, 2014 at 3:29 AM, Florian Haas <flor...@hastexo.com> wrote:
> On Thu, Jan 23, 2014 at 7:22 PM, Ben Nemec <openst...@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" <openst...@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@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev