sqlalchemy/devel

SQLAlchemy weekly dev meetings: https://docs.google.com/document/d/1WuzZwV_aIV_ltDR3cJ-bXxXMZ9zCfr5pFSXZGZOSSQw/edit?usp=sharing

Federico Caselli

@CaselIT

ok this works

    def _default_identity(self, autogen_context, col_type):
        if self._reflected_identity_default is not None:
            return self._reflected_identity_default
        nested = autogen_context.connection.begin_nested()
        try:
            name = "alembic_temp_table_%s" % id(self)
            meta = MetaData()
            Table(name, meta, Column("c", col_type, sqla_compat.Identity()))
            meta.create_all(autogen_context.connection)
            meta2 = MetaData()
            table2 = Table(name, meta2)
            sqla_compat._reflect_table(autogen_context.inspector, table2, None)
            self._reflected_identity_default = table2.c.c.identity
            self._reflected_identity_default.column = None
            return self._reflected_identity_default
        finally:
            nested.rollback()

but is very ugly
@zzzeek thoughts

mike bayer

@zzzeek

@CaselIT clearly im not following this identity thing nor much else lately

@CaselIT is that ...creating a table to try to detect something ?

i wouldnt do that...:)

nobody is going to appreciate that happening

Federico Caselli

@CaselIT

Indeed it is. I too would not do that. Comparing existing identity is becoming a can of worms. I think I'm going to scale back the ambition and just detect creation / removal of it for the moment. A more intelligent logic can follow at a second time

mike bayer

@zzzeek

@CaselIT +1

Federico Caselli

@CaselIT

will do

an alternative is to configure the defaults in the code somewhere, but it's a bit long, since in will depend by the type / direction of the identity

mike bayer

@zzzeek

I'm not really hearing anything about identity from the userbase yet besides the user that brought up this whole thing in the 1st place so I dont think we have to go too crazy right now :)

as always the gigantic raging fire in alembic is the enum thing :)

never seen such a small area get so many requests for improvement

Federico Caselli

@CaselIT

Ahah. Enum are a bit of a mess also in pg. Like you cannot remove elements from it

mike bayer

@zzzeek

yup

they suck

but also to add elements, you have to turn autocommit on

i have notions of migration files getting big conditioanl structures in them

that raise by default until the user goes in and gives the green light they are OK entering autocommit

Federico Caselli

@CaselIT

Out of curiosity mike, would there a more appropriate way of doing the check that I tried above? I mean other than opening a transaction, creating a table, reflecting it and rolling back?

mike bayer

@zzzeek

I lack the context to know what it is you actually want to do. is it that an Identity on our end has no arguments, and you wish to compare what would be the "default" of that object with the one in the DB ?

Federico Caselli

@CaselIT

Yes

Basically the use case is: user set a parameter, like increment to 2; creates the table; reset the increment to None.

mike bayer

@zzzeek

in my view, if someone put an Identity() in their model with specific defaults, then created the DB, then removed the arguments from the Identity and made it blank, then ran autogen, I would not expect it to know whether or not the database should change. if the user model lacks specificity then it follows they should not expect alembic to recreate that specificity

this is the same as what we do when comparing types

Federico Caselli

@CaselIT

I'm coming to that idea also. As the alternative needs the db side default.

mike bayer

@zzzeek

if I put "Number()" in my model

and the DB that means DECIMAL(10, 2)

we don't try to detect that it;s DECIMAL(10, 2) and not DECIMAL(10, 3)

Federico Caselli

@CaselIT

Ok. I can detect if user changes from increment=2 to increment=1

So I'll limit to that.

mike bayer

@zzzeek

it might be nice to add a new class of warnings that can be enabled for all the places where alembic makes a decision like this

Federico Caselli

@CaselIT

We can add a note somewhere and that's that I guess.

mike bayer

@zzzeek

ideally this would be documented and be positively identified progammatically, im referring mostly to the type comaprison logic but then places like this also

people would like that, to see where laembic is like, "hey i cant tell if these are different btw"

it would be a very noisy warning, so like a -vv kind of flag maybe

and we could even put a hook there

Federico Caselli

@CaselIT

I can add a warning of the type "ignoring comparisons of attributes since they are at default"

mike bayer

@zzzeek

@CaselIT yes but I am envisioning a more general API, like .ambiguous_comparison("type", model_obj, reflected_obj, "attribute X Y Z")

then there's some common API that different areas, like the part that compares Identity, the part that compares CheckConstraints, the part that comapres types, etc. can all call that thing

i.e. it's more interesting to me that we identify everywhere this happens

Federico Caselli

@CaselIT

We can add that to the list :)

Federico Caselli

@CaselIT

ok I'm making progress in the identity stuff. mostly tests left

Jonathan Vanasco

@jvanasco

@CaselIT is there a public api in sqlalchemy that shows what a column is reflected as (all the attributes) or is that code deeply nested in the logic that maps the output to a class/object?

Federico Caselli

@CaselIT

You mean a single column? I think there is for a table, this one https://docs.sqlalchemy.org/en/14/core/reflection.html?highlight=inspector#sqlalchemy.engine.reflection.Inspector.get_columns

This returns a lost of dicts, that is used by reflect_table

Jonathan Vanasco

@jvanasco

Thanks. I am thinking of generating a file that shows what creating columns with Identity() and Identity(increment=2) reflects as - under each database

This way we can pick up any changes in database versions on unit tests

If a new database version changes, chances are it won't affect anything - but at least we will know

Federico Caselli

@CaselIT

That would he useful

Where communities thrive

People

Repo info

Activity