Fix migrations in postgres

[gtfierro]

In the 542bfbdef624_constrain_dependencies_to_have_no_.py migration, we set autoincrement and primary key properties for the deps_association_table.id field. These should result in autoincrementing primary key behavior in all databases, but due to the way that we add/remove the constraints, the autoincrementing behavior does not get realized in a postgres database. This PR adjusts the migration (I believe this is correct but have not tested it on existing databases) and adds an integration test invoking the docker compose setup and inserting libraries into a migrated database instance

[TShapinsky]

This looks fine to me. I'd be interested in hearing @haneslinger 's thoughs since she has feelings about migrations. Also see my comment.

[TShapinsky]

we may want to merge our docker-compose fixtures at some point. Then we can have a subset of tests which need to run in that environment. This would allow us to use gh actions to handle uping and building the containers. And we could also do it once instead of redeploying them for every test.

[gtfierro]

I like the idea, though to make each of the tests independent we will probably want to wipe any state anyway. Can you make an issue to handle this in the future?

[haneslinger]

Adding a non-nullable column without a default value or something for the existing rows typically fails. Did you test to on a db with deps_associations in it?

[gtfierro]

I spent a lot of time yesterday trying to get that set up to test but I could never get it to work. I also don't think there are any existing BuildingMOTIF databases out there that would use this migration --- is it worth just removing this migration and just putting the changes directly into BuildingMOTIF?

Essentially, I think the engineering effort in getting this migration to work properly (and getting it properly tested) is outweighing the benefit of having it in there. Given that there aren't really any existing BuildingMOTIF deployments at this stage, I would much rather take the easy way out :)

[haneslinger]

You are coming in this afternoon, right? I figured out the problem, I'd like to explain it to you

[gtfierro]

Yep! See you soon

-- Dr. Gabe Fierro | https://home.gtf.fyi<https://home.gtf.fyi/> Assistant Professor of Computer Science | Colorado School of Mines Joint Appointment | National Renewable Energy Laboratory

________________________________ From: Hannah Eslinger ***@***.***> Sent: Tuesday, June 13, 2023 10:25:01 AM To: NREL/BuildingMOTIF ***@***.***> Cc: Gabe Fierro ***@***.***>; Author ***@***.***> Subject: [EXTERNAL] Re: [NREL/BuildingMOTIF] Fix migrations in postgres (PR #247) CAUTION: This email originated from outside of the Colorado School of Mines organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe. @haneslinger commented on this pull request.

________________________________ In migrations/versions/542bfbdef624_constrain_dependencies_to_have_no_.py<#247 (comment)>:

batch_op.drop_constraint("deps_association_table_pkey", type_="primary")

- batch_op.create_primary_key("deps_association_pk", columns=["id"]) - batch_op.alter_column( - existing_type=sa.INTEGER(), - column_name="id", - autoincrement=True, - existing_autoincrement=True, - nullable=False, - ) + batch_op.add_column(sa.Column("id", sa.Integer(), primary_key=True)) You are coming in this afternoon, right? I figured out the problem, I'd like to explain it to you — Reply to this email directly, view it on GitHub<#247 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAANRCT26YEWUFIDF3NH4WLXLCH53ANCNFSM6AAAAAAYIYYS4A>. You are receiving this because you authored the thread.Message ID: ***@***.***>