<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Jay,<br>
<br>
Comments below..<br>
<br>
Thanks,<br>
Octave<br>
<br>
<div class="moz-cite-prefix">On 7/23/2017 4:10 PM, Jay Pipes wrote:<br>
</div>
<blockquote
cite="mid:7973c763-a911-9a27-4522-1d4bac849ebb@gmail.com"
type="cite">Glad you brought this up, Mike. I was going to start a
thread about this. Comments inline.
<br>
<br>
On 07/23/2017 05:02 PM, Michael Bayer wrote:
<br>
<blockquote type="cite">I've been working with Octave Oregon in
assisting with new rules and
<br>
datatypes that would allow projects to support the NDB storage
engine
<br>
with MySQL.
<br>
<br>
To that end, we've made changes to oslo.db in [1] to support
this, and
<br>
there are now a bunch of proposals such as [2] [3] to implement
new
<br>
ndb-specific structures in projects.
<br>
<br>
The reviews for all downstream projects except Cinder are still
under
<br>
review. While we have a chance to avoid a future naming problem,
I am
<br>
making the following proposal:
<br>
<br>
Rather than having all the projects make use of
<br>
oslo_db.sqlalchemy.ndb.AutoStringTinyText / AutoStringSize, we
add new
<br>
generic types to oslo.db :
<br>
<br>
oslo_db.sqlalchemy.types.SmallString
<br>
oslo_db.sqlalchemy.types.String
<br>
</blockquote>
<br>
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. :)
<br>
<br>
<blockquote type="cite">(or similar )
<br>
<br>
Internally, the ndb module would be mapping its implementation
for
<br>
AutoStringTinyText and AutoStringSize to these types.
Functionality
<br>
would be identical, just the naming convention exported to
downstream
<br>
consuming projects would no longer refer to "ndb.<name>"
for
<br>
datatypes.
<br>
<br>
Reasons for doing so include:
<br>
<br>
1. openstack projects should be relying upon oslo.db to make the
best
<br>
decisions for any given database backend, hardcoding as few
<br>
database-specific details as possible. While it's unavoidable
that
<br>
migration files will have some "if ndb:" kinds of blocks, for
the
<br>
datatypes themselves, the "ndb." namespace defeats
extensibility.
<br>
</blockquote>
<br>
Right, my thoughts exactly.
<br>
<br>
<blockquote type="cite">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.
<br>
</blockquote>
<br>
Yep.
<br>
<br>
<blockquote type="cite">2. The names "AutoStringTinyText" and
"AutoStringSize" themselves are
<br>
confusing and inconsistent w/ each other (e.g. what is "auto"?
one is
<br>
"auto" if its String or TinyText and the other is "auto" if its
<br>
String, and..."size"?)
<br>
</blockquote>
<br>
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.
<br>
</blockquote>
<br>
FYI, the TINYTEXT is part of the MySQL syntax and dialect. So it's
not alien to MySQL folks.<br>
<blockquote
cite="mid:7973c763-a911-9a27-4522-1d4bac849ebb@gmail.com"
type="cite">
<br>
<blockquote type="cite">3. it's not clear (I don't even know right
now by looking at these
<br>
reviews) when one would use "AutoStringTinyText" or
"AutoStringSize".
<br>
For example in
<a class="moz-txt-link-freetext" href="https://review.openstack.org/#/c/446643/10/nova/db/sqlalchemy/migrate_repo/versions/216_havana.py">https://review.openstack.org/#/c/446643/10/nova/db/sqlalchemy/migrate_repo/versions/216_havana.py</a><br>
I see a list of String(255)'s changed to one type or the other
without
<br>
any clear notion why one would use one or the other. Having
names
<br>
that define simply the declared nature of the type would be most
<br>
appropriate.
<br>
</blockquote>
<br>
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 ;)
<br>
</blockquote>
<br>
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.<br>
<br>
<blockquote
cite="mid:7973c763-a911-9a27-4522-1d4bac849ebb@gmail.com"
type="cite">
<br>
<blockquote type="cite">I can add these names up to oslo.db and
then we would just need to
<br>
spread these out through all the open ndb reviews and then also
patch
<br>
up Cinder which seems to be the only ndb implementation that's
been
<br>
merged so far.
<br>
</blockquote>
<br>
+1
<br>
<br>
<blockquote type="cite">Keep in mind this is really me trying to
correct my own mistake, as I
<br>
helped design and approved of the original approach here where
<br>
projects would be consuming against the "ndb." namespace.
However,
<br>
after seeing it in reviews how prevalent the use of this
extremely
<br>
backend-specific name is, I think the use of the name should be
much
<br>
less frequent throughout projects and only surrounding logic
that is
<br>
purely to do with the ndb backend and no others. At the
datatype
<br>
level, the chance of future naming conflicts is very high and we
<br>
should fix this mistake (my mistake) before it gets committed
<br>
throughout many downstream projects.
<br>
</blockquote>
<br>
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. :)
<br>
<br>
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.
<br>
<br>
My complaint regarding the code patch proposed to Nova was around
the hard-coding of the ndb namespace into the model definitions.
<br>
<br>
Best,
<br>
-jay
<br>
<br>
<blockquote type="cite">
<br>
[1] <a class="moz-txt-link-freetext" href="https://review.openstack.org/#/c/427970/">https://review.openstack.org/#/c/427970/</a>
<br>
<br>
[2] <a class="moz-txt-link-freetext" href="https://review.openstack.org/#/c/446643/">https://review.openstack.org/#/c/446643/</a>
<br>
<br>
[3] <a class="moz-txt-link-freetext" href="https://review.openstack.org/#/c/446136/">https://review.openstack.org/#/c/446136/</a>
<br>
<br>
__________________________________________________________________________
<br>
OpenStack Development Mailing List (not for usage questions)
<br>
Unsubscribe:
<a class="moz-txt-link-abbreviated" href="mailto:OpenStack-dev-request@lists.openstack.org?subject:unsubscribe">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a>
<br>
<a class="moz-txt-link-freetext" href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
<br>
<br>
</blockquote>
<br>
__________________________________________________________________________
<br>
OpenStack Development Mailing List (not for usage questions)
<br>
Unsubscribe:
<a class="moz-txt-link-abbreviated" href="mailto:OpenStack-dev-request@lists.openstack.org?subject:unsubscribe">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a>
<br>
<a class="moz-txt-link-freetext" href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
<br>
</blockquote>
<br>
<div class="moz-signature"><br>
<font color="#666666" size="2" face="Verdana, Arial, Helvetica,
sans-serif"><font color="#666666" size="2" face="Verdana, Arial,
Helvetica, sans-serif"></font></font></div>
</body>
</html>