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

Find a way to prevent coupling future migrations with business domain code by mistake #8540

Open
acelaya opened this issue Feb 20, 2024 · 2 comments

Comments

@acelaya
Copy link
Contributor

acelaya commented Feb 20, 2024

Most of the migrations define their own models to avoid importing from the rest of the source code, but as seen in #8541, it is possible to make a mistake.

It would be useful to have some automated way to highlight when a migration is importing business domain code, as this is usually an overlook.

One option is a linting rule. This package seems to go in that direction https://pypi.org/project/import-linter/, but it would be good to be able to do this with the existing tooling.

@acelaya acelaya transferred this issue from hypothesis/lms Feb 20, 2024
@robertknight
Copy link
Member

I suggest to redefine this issue more narrowly as: Enforce that schema migrations in h/migrations don't import from h.models, because the migrations will have been written against a schema that is different than the current schema. A mismatch between the DB schema and SQLAlchemy model fields can cause the migration to fail to run.

For context, see https://hypothes-is.slack.com/archives/C4K6M7P5E/p1708419269758879.

@acelaya
Copy link
Contributor Author

acelaya commented Feb 20, 2024

I suggest to redefine this issue more narrowly as: Enforce that schema migrations in h/migrations don't import from h.models, because the migrations will have been written against a schema that is different than the current schema. A mismatch between the DB schema and SQLAlchemy model fields can cause the migration to fail to run.

I think they should actually not import anything which is not the standard library or tooling provided by the migrations library.

The issue reported in #8541 was caused specifically by a SQLAlchemy model that changed, but any other change could cause errors if it is used by a migration: Enums which values change, functions which are refactored/deleted/renamed, etc

Migrations should be as isolated as possible, in my opinion.

@acelaya acelaya changed the title Find a way to avoid coupling migrations with business domain code Find a way to prevent coupling future migrations with business domain code by mistake Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants