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

feat: Move alembic config into mealie package for easier distribution #4329

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

chishm
Copy link
Contributor

@chishm chishm commented Oct 7, 2024

What type of PR is this?

  • cleanup
  • feature

What this PR does / why we need it:

The Alembic configuration and database migration scripts are moved inside the mealie Python package. This means they will be always available, in a consistent location. The advantages are:

  • No extra step needed during mealie installation to add the Alembic configuration and scripts.
  • No need to guess where the configuration is, so the ALEMBIC_CONFIG_FILE environment variable doesn't need to be set, ever. I've left it in as an option for people who still want to use it.

As a side effect, ruff (specifically isort) no longer thinks alembic is a project ("LOCALFOLDER") package, so the imports of alembic are moved to the third-party group.

Which issue(s) this PR fixes:

This is one task from discussion #4322.

Testing

I built and ran the Docker container, then made sure the database migration still occurred.

I created a database change following the instructions in database-changes.md.

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the stale label Nov 22, 2024
@chishm
Copy link
Contributor Author

chishm commented Nov 22, 2024

I'm waiting on #4551 to be reviewed and merged before I rebase this PR for merge.

But if someone wants to review this one first, I'm happy to rebase now.

@github-actions github-actions bot removed the stale label Nov 23, 2024
@hay-kot hay-kot self-requested a review December 3, 2024 21:27
hay-kot
hay-kot previously approved these changes Dec 3, 2024
Copy link
Collaborator

@hay-kot hay-kot left a comment

Choose a reason for hiding this comment

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

LGTM, please rebase and I'll get it merged!

@chishm
Copy link
Contributor Author

chishm commented Dec 4, 2024

Thank you for the review. I've rebased and re-run the manual tests (creating a database migration, building and running the Docker image) and I'm happy for this to be merged.

@hay-kot hay-kot merged commit a6cbf13 into mealie-recipes:mealie-next Dec 4, 2024
13 checks passed
@tecosaur
Copy link

A quick question, after this change does this patch still look relevant? https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/me/mealie/package.nix#L94-L108

@chishm
Copy link
Contributor Author

chishm commented Dec 17, 2024

A quick question, after this change does this patch still look relevant? https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/me/mealie/package.nix#L94-L108

Taking a look, I think lines 95-98 are still relevant, but 100-107 shouldn't be necessary any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants