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

Only make a connection to the DB if necessary #255

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mjvankampen
Copy link

When updating to Django 4 I noticed that suddenly our CI failed as there was no valid database while linting migrations. This change will only connect to a DB when it is necessary (i.e. check only applied or unapplied migrations). That way you don't have to do trickery in your settings file when you run the linter without a DB.

@mjvankampen
Copy link
Author

Ah sorry not related to Django4 but still I think this PR is valid.

@mjvankampen mjvankampen closed this May 9, 2023
@mjvankampen mjvankampen deleted the no-connection branch May 9, 2023 09:00
@mjvankampen mjvankampen restored the no-connection branch May 9, 2023 09:12
@mjvankampen mjvankampen reopened this May 9, 2023
Copy link
Collaborator

@David-Wobrock David-Wobrock left a comment

Choose a reason for hiding this comment

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

Hey @mjvankampen ! Thanks for contributing and taking interest in the project 🙇

I'm not sure I get the rationale behind this change 🤔
We still require a DB connection even when we don't specify only applied or unapplied migrations. The MigrationLoader object is still required to load and lint migrations.
I'm not sure it can work with this new condition.

Do you have more context/information about your case? :)

@David-Wobrock David-Wobrock self-assigned this Jul 2, 2023
@mjvankampen
Copy link
Author

mjvankampen commented Jul 15, 2023

Sorry I’m somewhat new to databases and Django (coming from an embedded environment). While setting up CI/CD I noticed that to lint migrations you need to connect to the database. On a higher level I don’t really understand that as you don’t need to know the current state of the database as that is captured in your migrations already. Except if you only want to lint unapplied migrations for example.

From your reply the MigrationLoader is needed to lint the migrations and the MigrationLoader needs a connection to a database (the state of the db does not matter?) which is I guess hard to get around.

Ps. Love the project and its helped me capture some bad migrations already!

Please let me know if I understand correctly, then I’ve learned something and will work around it (like spin up a DB for linting).

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

2 participants