On 10/07/2016 02:36 AM, Chris Withers wrote:
On 06/10/2016 15:04, Mike Bayer wrote:
So Mortar seems to be a heavily SQLAlchemy-integrated product.
It has a very small user base ;-)
I'd like
to point out that whenever we do these year-long release cycles, I put
out a lot of betas very early on that hopefully contain all the
biggish
changes. This one has been available on pypi since June 16.
Nobody has
said anything about it.
Right, unfortunately my testing strategy is generally to use the latest
official releases (ie: what "pip install x" would pick) so as not to
get
a lot of noise from the nightlies.
With SQLAlchemy, I'd happily make an exception, but how do I change my
travis.yml to have an axis for SQLAlchemy version that is "latest
stable" and "latest beta"?
there's a pip flag at that level. I use jenkins myself but if we ask
the usual suspects on twitter we can probably find an answer.... :)
So, this code path used to previously raise an IntegrityError and now
raises a CompileError. What was the driver for that? I can certainly
see
there being code in the wild that was expecting an IntegrityError and
will now fail...
The change itself, including the text of that exception verbatim so
that
google searches can find it, is described in detail here:
http://docs.sqlalchemy.org/en/latest/changelog/migration_11.html#the-autoincrement-directive-is-no-longer-implicitly-enabled-for-a-composite-primary-key-column
.
Right, that change seems sensible here, although not sure
I wouldn't have it raise that CompileError if I wasn't changing the API
on a very small set of users. The suffering spread by the old way
autoincrement worked had to come to an end.
There's definitely code that looks for IntegrityError in the wild,
however such code (such as what I've written for Openstack) is
doing so
in order to detect conditions that cannot be caught beforehand,
such as
a row that conflicts with a unique constraint or a row that is
rejected
as a concurrent update.
In this case, you have a Table that has been configured to not expect
the database to generate any kind of identifier for a primary key
column
(which is the change here, previously, it was),
Sorry, don't follow the bit in brackets; the test model has no integer
columns.
present in the INSERT statement. So it is known ahead of time that
this
will either fail unconditionally, or the Table is
mis-configured. The
change is specifically to alert users that this is now a
mis-configuration. E.g. so that I wouldn't get a "regression!" email
:) .
I think I follow that, but my test is verifying that:
session.add(Model(period='...'))
session.save()
(nb: Model is erroneously instantiated with no string primary key)
...gives an IntegrityError, which I think it what most people would
expect. There's no mis-configuration there, it's just a bad object
which, if inserted, would result in a row with now primary key.
Right this is the "it will fail unconditionally" choice. I agree
sending to the DB and getting the error back is less mysterious. But I
don't see a way to distinguish between someone who was relying upon
implicit autoincrement for composite PK and who isn't. I can just flip
my mind and say, OK, your scenario is way more likely than the other
one, and we flip. Then I get people saying "why are there
integrityerrors all the sudden w/ no explanation?" . Sure, I'll make
it a warning. Release will be today. Please test w/ the betas next
time.
There is existing precedent for SQLAlchemy catching a NULL primary key
value before the INSERT is actually emitted, the ORM has done this
for a
long time.
This is for the identity map, right? That's interesting, what exception
does it raise in that circumstance? (More out of curiosity than
anything: I'm wondering why that exception wasn't raised in this test
case and I ended up with an IntegrityError)
cheers,
Chris
