alembic_utils icon indicating copy to clipboard operation
alembic_utils copied to clipboard

Add mssql view support

Open mortenbpost opened this issue 4 years ago • 5 comments

Hey,

PR for adding MSSQL View support - it's not polished yet but fully functional and working. I've obviously hit a few bumps in the road because it seems it wasn't originally created with the idea that it should support anything but PG?

So this PR is mainly to get an idea whether you could be interested in adding this functionality to alembic_utils and also to get some feedback if you want.

I've also changed a few things which I believe could be bugs. The one that causes the most issues was creating a new connection in register_entities which caused a lock issue when running tests on mssql due to the fact that the migrations and this was both querying information_schema.views.

mortenbpost avatar Jan 26 '21 14:01 mortenbpost

In addition to the alembic_utils test suite, I'm able to test postgres support on a multi-hundred entity codebase. That testing is often where subtle bugs are surfaced. Since I don't have an equivalent mssql project to test against, I couldn't be as confident in the quality of the mssql support at this point.

That said, I don't have any immediate plans to make significant changes to the ReplaceableEntity interface. If you'd like to publish mssql view support as a separate package, e.g. alembic_utils_mssql, I'd be happy to link to it from the README and work with you to maintain compatibility.

Longer term I'd be open to making it an optional dependency (alembic_utils[mssql]) or even merging back into alembic_utils once its been more extensively tested

olirice avatar Jan 26 '21 15:01 olirice

Thanks.

It makes sense. We will soon have this running on something that's closer to 50-100 entity codebase. Once I'm confident enough that it works and when I've figured out if it's something I want to maintain I'll get back to you. I will not rule out maintaining a separate package.

Thanks for creating this package :-)

mortenbpost avatar Jan 27 '21 10:01 mortenbpost

@mortenbpost great work here. I have actually forked this package @olirice to implement views in MySQL (note @mortenbpost is using MSSQL - microsoft sql server) and am using it on a database with 100s of entities. I noticed that @olirice was making a lot of updates recently and had considered writing a PR back to this, too, to keep things in sync.

The changes required are similar to those made by @mortenbpost here - dialect specific changes to how views, etc., are created, etc.. I also noticed a slight change required in the event listener compare_registered_entities to support multiple databases (i.e. multiple sqlalchemy bases) from a single implementation, which is another unique characteristic of the system I am supporting.

Let me know @olirice your interest level in those kind of additions; from this conversation, a separate package similarly seems like your desire.

Let me echo my thanks for your awesome work on this package.

andersberliner avatar Jan 30 '21 05:01 andersberliner

Re: mysql/mssql: whoops, thanks for the correction. Gotta work on those reading skills!

I'd be interested to hear specifics about the changes you had to make to compare_registered_entities. Ideally it should not contain anything postgres specific.

olirice avatar Jan 30 '21 20:01 olirice

@andersberliner I'd also be interested in the specifics related to compare registered entities.

Just updated the branch so it's based on the most recent changes - was straight forward.

mortenbpost avatar Feb 03 '21 11:02 mortenbpost

closing for age

olirice avatar Feb 15 '23 21:02 olirice