Skip to content

Alembic triggers #580

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

AcquaDiGiorgio
Copy link

With this PR I want to display the possibility to create and drop triggers using Alembic.

I left some code that achieves the same thing using just SQLAlchemy commented to have something to compare them, but they must not work along side each other.

Known Issues

  • Currently MySQL specific. PostgreSQL is the only language to have a special package to deal with this issues. For other dialects, including MySQL, it doesn't seem to be anything similar and it has to be done this way.

  • Auto-generation of certain DiracX specific data types (not the Triggers) have some problems. I have seen this behaviour with the enums, which it doesn't know how the init function works.

  • The url for the connection must be from a privileged user, so we cannot use the environment variable obtained from BaseSQLDB.available_urls()["DB_NAME"].

  • The Alembic directory structure of the migrations has been initialized only for the JobDB database, but it can be either copied to other databases, or we can try to have a singular alembic script and multiple migrations. Apparently is not easy or recommended, but there are guides, such as this one, that makes it look simple.

Recommended documentation

Alembic Plugins: Explains how to create new operations for not implemented SQL statements such as CREATE TRIGGER.

Alembic Autogeneration: Shows how to make Alembic know how to generate the code of the plugins.

GitHub Issue - PG Triggers: Someone had a similar issue with PostgreSQL and the comments illustrate how to modify alembic's context to automatically generate code using the directives in the .mako template file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding, is this file created by alembic?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I named them 'init' and 'new', but they get created when you do alembic revision --autogenerate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can they stay in a specific directory, e.g. diracx-db/src/diracx/db/sql/_generated ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! There are 2 configuration variables in the alembic.ini file called script_location and version_locations that set up both the location of the Alembic environment and its revisions.

  • script_location defaults to %(here)s/migrations, where %(here)s is the location of the .ini.
  • version_locations defaults to <script_location>/versions but it can also be configured.

# ### end Alembic commands ###


def downgrade() -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems quite ... wrong!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What part in particular? The table dropping?
If that's the case, remember that this is the very first version of the database and the previous state is an empty one, so it drops everything it created.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's certainly not what we want. The version we have is "inherited" from DIRAC so no dropping!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we will never downgrade? And for upgrades, if a table is removed from the schema, does it need to stay in the DB?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to downgrade at some point (or at least having the possibility to do so).

For the upgrades: if a table is removed from the schema it does not need to stay in the DB.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the only change would be that the initial version has no downgrade?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so.

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.

2 participants