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

implement alembic to handle database migrations #2317

Closed
wants to merge 1 commit into from

Conversation

RustyBower
Copy link
Contributor

Description

Implement Alembic to handle current and future database migrations

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@RustyBower
Copy link
Contributor Author

I do need to figure out how to automatically run Alembic for new versions of Sopel 🤔

@dgw
Copy link
Member

dgw commented Jul 13, 2022

That'd be cool. I'm not clear on how this works with packaged installs vs. being installed from Git, because the version appears based on commit SHAs, but also haven't read this whole patch yet (nor read Alembic docs).

Maybe later when I'm not out and on a phone. 😁

@RustyBower RustyBower added the Breaking Change Stuff that probably should be mentioned in a migration guide label Jul 13, 2022
@Exirel
Copy link
Contributor

Exirel commented Jul 13, 2022

As said on IRC, I'm sorry I've to say no, because I've been there, I've done the work on previous project so I know what it means. However when we want migrations in Sopel:

  • it needs to be a sopel command line, allowing to select a different configuration file
  • it needs to allow us to generate new migrations that can be packaged
  • it needs to allow Sopel's owner to run migrations that was packaged
  • it needs to allow plugin's author to generate new migrations for their plugin (in that case, maybe only packaged version?)
  • it needs to collect plugin's migrations to run them alongside Sopel's own migrations

Basically, we don't need the default behavior of Alembic, which is to run for a project you 100% control, we need a system closer to what Django's ORM support: generate migrations for your project, package it, then collect all migrations from the project and its installed plugin to run them. And trust me, I know it's not easy, I had to done it once a few years ago and it wasn't fun at all.

As a side note, the migration needs to have a complete downgrade handler, this one is incomplete.

@RustyBower
Copy link
Contributor Author

There's definitely a way to package this within the application itself to migrate on boot / startup, since this is functionality that Flask-Migrate has, so it must be possible.

@Exirel
Copy link
Contributor

Exirel commented Jul 13, 2022

I know there is a way: I've done it. I also know this PR is not the way.

@RustyBower
Copy link
Contributor Author

I'm open to other suggestions

@dgw
Copy link
Member

dgw commented Feb 28, 2023

This seems to have well and truly stalled. Since it didn't seem to be the right approach anyway, let's take any future discussion back to #2293. I'll copy over @Exirel's list of requirements.

@dgw dgw closed this Feb 28, 2023
@dgw dgw added the Declined Requests that will not be implemented for technical or project direction reasons label Feb 28, 2023
@dgw dgw deleted the alembic_upgrade branch February 28, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Stuff that probably should be mentioned in a migration guide Declined Requests that will not be implemented for technical or project direction reasons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants