Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make PyIntEnum freeze enums in Alembic revisions #4425

Merged
merged 2 commits into from May 6, 2020

Conversation

OmeGak
Copy link
Member

@OmeGak OmeGak commented Apr 23, 2020

No description provided.

@OmeGak OmeGak force-pushed the wip/frozen-pyintenum branch 2 times, most recently from 5fbdc89 to fee8918 Compare Apr 23, 2020
@OmeGak OmeGak changed the title Freeze PyIntEnum objects in Alembic revisions Freeze PyIntEnum classes in Alembic revisions Apr 23, 2020
@OmeGak OmeGak force-pushed the wip/frozen-pyintenum branch 2 times, most recently from 78cbaf2 to f31423d Compare Apr 23, 2020
@OmeGak OmeGak changed the title Freeze PyIntEnum classes in Alembic revisions Make PyIntEnum freeze enums in Alembic revisions Apr 23, 2020
indico/core/db/sqlalchemy/custom/int_enum.py Outdated Show resolved Hide resolved
indico/core/db/sqlalchemy/custom/int_enum.py Outdated Show resolved Hide resolved
indico/core/db/sqlalchemy/custom/int_enum.py Outdated Show resolved Hide resolved
@mic4ael
Copy link
Member

mic4ael commented Apr 27, 2020

Out of curiosity, what problem are you trying to solve here? In which case importing the Enum directly wouldn't suffice?

@ThiefMaster
Copy link
Member

ThiefMaster commented Apr 27, 2020

Imagine changes like this:

  • rev n: create enum column
  • rev n+x: add new member to enum

Now you update from a version that was at rev n-1 to one that has at least n+x. When running rev n it will create the check constraint for the enum with all fields the enum has, even the ones that were added later.

Technically this is not a problem since it'll make the change in n+x just a no-op, but it means this revision is no longer idempotent, because if you upgrade and downgrade it, you will not have the same database structure you had before it (since the downgrade will remove the extra values from the check constraint)

@mic4ael
Copy link
Member

mic4ael commented Apr 27, 2020

Imagine changes like this:

* rev n: create enum column

* rev n+x: add new member to enum

Now you update from a version that was at rev n-1 to one that has at least n+x. When running rev n it will create the check constraint for the enum with all fields the enum has, even the ones that were added later.

Technically this is not a problem since it'll make the change in n+x just a no-op, but it means this revision is no longer idempotent, because if you upgrade and downgrade it, you will not have the same database structure you had before it (since the downgrade will remove the extra values from the check constraint)

Thanks! Didn't think of such case.

@OmeGak
Copy link
Member Author

OmeGak commented Apr 27, 2020

Technically this is not a problem since it'll make the change in n+x just a no-op, but it means this revision is no longer idempotent, because if you upgrade and downgrade it, you will not have the same database structure you had before it (since the downgrade will remove the extra values from the check constraint)

To complete the explanation, this is relevant in the context of the idempotency checks that we implemented for the UN plugin.

@mic4ael
Copy link
Member

mic4ael commented Apr 28, 2020

Technically this is not a problem since it'll make the change in n+x just a no-op, but it means this revision is no longer idempotent, because if you upgrade and downgrade it, you will not have the same database structure you had before it (since the downgrade will remove the extra values from the check constraint)

To complete the explanation, this is relevant in the context of the idempotency checks that we implemented for the UN plugin.

Unfortunately I have no access to this repository :(

@OmeGak OmeGak force-pushed the wip/frozen-pyintenum branch from 9b75cd5 to 5ad889a Compare May 6, 2020
@ThiefMaster ThiefMaster merged commit 5ad889a into indico:master May 6, 2020
5 checks passed
@ThiefMaster ThiefMaster deleted the wip/frozen-pyintenum branch May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants