Open Stack

Hi Jay,

Comments below..

Thanks,
Octave

On 7/23/2017 4:10 PM, Jay Pipes 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:
>> I've been working with Octave Oregon in assisting with new rules and
>> datatypes that would allow projects to support the NDB storage engine
>> with MySQL.
>>
>> To that end, we've made changes to oslo.db in [1] to support this, and
>> there are now a bunch of proposals such as [2] [3] to implement new
>> ndb-specific structures in projects.
>>
>> The reviews for all downstream projects except Cinder are still under
>> review. While we have a chance to avoid a future naming problem, I am
>> making the following proposal:
>>
>> Rather than having all the projects make use of
>> oslo_db.sqlalchemy.ndb.AutoStringTinyText / AutoStringSize, we add new
>> generic types to oslo.db :
>>
>> oslo_db.sqlalchemy.types.SmallString
>> oslo_db.sqlalchemy.types.String
>
> This is precisely what I was going to suggest because I was not going 
> to go along with the whole injection of NDB-name-specific column types 
> in Nova. :)
>
>> (or similar )
>>
>> Internally, the ndb module would be mapping its implementation for
>> AutoStringTinyText and AutoStringSize to these types. Functionality
>> would be identical, just the naming convention exported to downstream
>> consuming projects would no longer refer to "ndb.<name>" for
>> datatypes.
>>
>> Reasons for doing so include:
>>
>> 1. openstack projects should be relying upon oslo.db to make the best
>> decisions for any given database backend, hardcoding as few
>> database-specific details as possible.   While it's unavoidable that
>> migration files will have some "if ndb:" kinds of blocks, for the
>> datatypes themselves, the "ndb." namespace defeats extensibility.
>
> Right, my thoughts exactly.
>
>> if IBM wanted Openstack to run on DB2 (again?) and wanted to add a 
>> "db2.String" implementation to oslo.db for example, the naming and 
>> datatypes would need to be opened up as above in any case;  might as 
>> well make the change now before the patch sets are merged.
>
> Yep.
>
>> 2. The names "AutoStringTinyText" and "AutoStringSize" themselves are
>> confusing and inconsistent w/ each other (e.g. what is "auto"? one is
>> "auto" if its String or TinyText and the other is "auto" if its
>> String, and..."size"?)
>
> Yes. Oh God yes. The MySQL TINY/MEDIUM/BIG [INT|TEXT] data types were 
> always entirely irrational and confusing. No need to perpetuate that 
> terminology.

FYI, the TINYTEXT is part of the MySQL syntax and dialect. So it's not 
alien to MySQL folks.
>
>> 3. it's not clear (I don't even know right now by looking at these
>> reviews) when one would use "AutoStringTinyText" or "AutoStringSize".
>> For example in 
>> https://review.openstack.org/#/c/446643/10/nova/db/sqlalchemy/migrate_repo/versions/216_havana.py
>> I see a list of String(255)'s changed to one type or the other without
>> any clear notion why one would use one or the other.  Having names
>> that define simply the declared nature of the type would be most
>> appropriate.
>
> 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 ;)

Unfortunately, if we don't modify the scripts, we can't create the 
schemas on the NDB database. Tables have to fit in the row limits. So 
unless we have a way to override the scripts, we have to modify them.

>
>> 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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20170724/10265f7b/attachment.html>