Misleading warning about MySQL TRADITIONAL mode not being set

[Florian Haas]

common.db.sqlalchemy.session logs a scary warning if create_engine is not being called with mysql_traditional_mode set to True:

WARNING keystone.openstack.common.db.sqlalchemy.session [-] This application has not enabled MySQL traditional mode, which means silent data corruption may occur. Please encourage the application developers to enable this mode.

That warning is problematic for several reasons:

(1) It suggests the wrong mode. Arguably TRADITIONAL is better than the default, but STRICT_ALL_TABLES would actually be more useful.
(2) The user has no way to fix the warning.
(3) The warning does not take into account that a global sql-mode may in fact have been set via the server-side MySQL configuration, in which case the session *may* in fact be using TRADITIONAL mode all along, despite the warning saying otherwise. This makes (2) even worse.

My suggested approach would be:
- Remove the warning.
- Make the SQL mode a config option.

Patches forthcoming.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/68474

[Florian Haas (fghaas)]

Hey Doug,

thanks for looking into this -- your -1 is currently blocking
https://review.openstack.org/#/c/68473/ which the (approved) change that
fixes this depends on. I believe I've addressed your concerns in the
inline comments -- would you mind taking another look?

Thanks,
Florian

[Florian Haas (fghaas)]

Patch set abandoned because https://review.openstack.org/#/c/68693/ rendered the proposed fix moot.

We're back to square one on this one, and I have zero time to work on this in the next two weeks. If anyone else feels inclined to pick this up, please so.

[Florian Haas (fghaas)]

That was meant to be "please do", of course.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/68474
Committed: https://git.openstack.org/cgit/openstack/oslo-incubator/commit/?id=dda24eb4a815914c29e801ad0176630786db8734
Submitter: Jenkins
Branch: master

commit dda24eb4a815914c29e801ad0176630786db8734
Author: Florian Haas <email address hidden>
Date: Wed Jan 22 20:52:36 2014 +0100

    Introduce mysql_sql_mode option, remove old warning

    Introduce a config option, mysql_sql_mode, that allows the user
    to set the desired SQL mode for MySQL sessions.

    For MySQL sessions, set the SQL mode session variable
    accordingly.

    For compatibility purposes, retain the mysql_traditional_mode
    argument in method signatures, but only use it to override the
    config option as if it were set to 'TRADITIONAL'.

    Change-Id: Ia527e850671ab9542db5864617384cee21e91bf6
    Closes-bug: 1271706

[Brant Knudson (blk-u)]

Fix proposed to keystone: https://review.openstack.org/#/c/78429/

But it's blocked by excessive logging: https://bugs.launchpad.net/oslo/+bug/1288443 .

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/78429
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=d75c707886819a741d14218f85a4cae931f13467
Submitter: Jenkins
Branch: master

commit d75c707886819a741d14218f85a4cae931f13467
Author: Brant Knudson <email address hidden>
Date: Wed Mar 12 14:00:40 2014 -0500

    Sync db, db.sqlalchemy from oslo-incubator 0a3436f

    This change sync's oslo-incubator's db module from commit hash
    0a3436fbcd69b7e0cd1a768be15cbf794c803e3b

    $ python update.py --nodeps --base keystone \
        --dest-dir ../keystone \
        --modules db,db.sqlalchemy

    This includes a fix where the keystone server would log a warning
    that starts with

      This application has not enabled MySQL traditional mode ...

    This change includes the following commits from oslo-incubator:

    a1a8280 Fix excessive logging from db.sqlalchemy.session
    dc2d829 Add lockutils fixture to OpportunisticTestCase
    d10f871 Adapt DB provisioning code for CI requirements
    5920bed Make db utils importable without migrate
    9933bdd Get mysql_sql_mode parameter from config
    96a2217 Prevent incorrect usage of _wrap_db_error()
    20a7510 Add from_config() method to EngineFacade
    fea119e Drop special case for MySQL traditional mode, update unit tests
    a584166 Make TRADITIONAL the default SQL mode
    5b9e9f4 Fix doc build errors in db.sqlalchemy

    The above list was generated by doing the following in oslo-incubator:

     $ git log --oneline --no-merges \
         6ba44fd..0a3436fbcd69b7e0cd1a768be15cbf794c803e3b \
         openstack/common/db/ openstack/common/db/sqlalchemy

    The keystone log shows that the last sync was 6ba44fd:

     $ git log -n1 --oneline --no-merges \
         keystone/openstack/common/db \
         keystone/openstack/common/db/sqlalchemy
     8f7b87b Sync db, db.sqlalchemy, gettextutils from oslo-incubator 6ba44fd

    Closes-Bug: #1271706
    Change-Id: If537ff5166b8e9a6fc18c570cdd2e44943faac9c

[gordon chung (chungg)]

fix sync'd with this patch: https://review.openstack.org/#/c/78132/

[Bogdan Dobrelya (bogdando)]

depends on BP https://blueprints.launchpad.net/oslo/+spec/oslo-db-lib

[Ben Nemec (bnemec)]

It shouldn't depend on oslo.db. This was fixed in the incubator version of the db code too, so all that's needed to fix it is to sync the latest version of that.

[Jeff Peeler (jpeeler-z)]

Heat currently uses oslo.db, however this change was synced pre-oslo.db in cfcbc994d3e5f887ea008e3cc2260c517f53d269 (https://review.openstack.org/#/c/76539/). Setting to fix released as it's been available since juno-1.

[Sean Dague (sdague)]

seems fixed in nova