Open Stack

On Sun, Jul 23, 2017 at 6:10 PM, Jay Pipes <jaypipes at gmail.com> wrote:
> Glad you brought this up, Mike. I was going to start a thread about this.
> Comments inline.
>
> On 07/23/2017 05:02 PM, Michael Bayer wrote:
> Well, besides that point (which I agree with), that is attempting to change
> an existing database schema migration, which is a no-no in my book ;)


OK this point has come up before and I always think there's a bit of
an over-broad kind of purism being applied (kind of like when someone
says, "never use global variables ever!" and I say, "OK so sys.modules
is out right ?" :)  ).

I agree with "never change a migration" to the extent that you should
never change the *SQL steps emitted for a database migration*.  That
is, if your database migration emitted "ALTER TABLE foobar foobar
foobar" on a certain target databse, that should never change.   No
problem there.

However, what we're doing here is adding new datatype logic for the
NDB backend which are necessarily different; not to mention that NDB
requires more manipulation of constraints to make certain changes
happen.  To make all that work, the *Python code that emits the SQL
for the migration* needs to have changes made, mostly (I would say
completely) in the form of new conditionals for NDB-specific concepts.
   In the case of the datatypes, the migrations will need to refer to
a SQLAlchemy type object that's been injected with the ndb-specific
logic when the NDB backend is present; I've made sure that when the
NDB backend is *not* present, the datatypes behave exactly the same as
before.

So basically, *SQL steps do not change*, but *Python code that emits
the SQL steps* necessarily has to change to accomodate for when the
"ndb" flag is present - this because these migrations have to run on
brand new ndb installations in order to create the database.   If Nova
and others did the initial "create database" without using the
migration files and instead used a create_all(), things would be
different, but that's not how things work (and also it is fine that
the migrations are used to build up the DB).

There is also the option to override the compilation for the base
SQLAlchemy String type does so that no change at all would be needed
to consuming projects in this area, but it seems like there is a need
to specify ndb-specific length arguments in some cases so keeping the
oslo_db-level API seems like it would be best.  (Note that the ndb
module in oslo_db *does* instrument the CreateTable construct globally
however, though it is very careful not to be involved unless the ndb
flag is present).




>
>> I can add these names up to oslo.db and then we would just need to
>> spread these out through all the open ndb reviews and then also patch
>> up Cinder which seems to be the only ndb implementation that's been
>> merged so far.
>
>
> +1
>
>> Keep in mind this is really me trying to correct my own mistake, as I
>> helped design and approved of the original approach here where
>> projects would be consuming against the "ndb." namespace.  However,
>> after seeing it in reviews how prevalent the use of this extremely
>> backend-specific name is, I think the use of the name should be much
>> less frequent throughout projects and only surrounding logic that is
>> purely to do with the ndb backend and no others.   At the datatype
>> level, the chance of future naming conflicts is very high and we
>> should fix this mistake (my mistake) before it gets committed
>> throughout many downstream projects.
>
>
> I had a private conversation with Octave on Friday. I had mentioned that I
> was upset I didn't know about the series of patches to oslo.db that added
> that module. I would certainly have argued against that approach. Please
> consider hitting me with a cluestick next time something of this nature pops
> up. :)
>
> Also, as I told Octave, I have no problem whatsoever with NDB Cluster. I
> actually think it's a pretty brilliant piece of engineering -- and have for
> over a decade since I worked at MySQL.
>
> My complaint regarding the code patch proposed to Nova was around the
> hard-coding of the ndb namespace into the model definitions.
>
> Best,
> -jay
>
>>
>> [1] https://review.openstack.org/#/c/427970/
>>
>> [2] https://review.openstack.org/#/c/446643/
>>
>> [3] https://review.openstack.org/#/c/446136/
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev