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

Enable django_checks check from django-test-migrations #1384

Open
sobolevn opened this issue Nov 26, 2020 · 9 comments
Open

Enable django_checks check from django-test-migrations #1384

sobolevn opened this issue Nov 26, 2020 · 9 comments

Comments

@sobolevn
Copy link
Member

https://github.com/wemake-services/django-test-migrations/blob/master/django_test_migrations/contrib/django_checks.py#L47

@skarzi are you interested in helping me out?

@skarzi
Copy link

skarzi commented Nov 27, 2020

sure! I can prepare the initial PR for this issue on the weekend

@sobolevn
Copy link
Member Author

Awesome, thanks!

@skarzi
Copy link

skarzi commented Nov 28, 2020

I have tested django-test-migrations from master branch with wemake-django-template on my fork issue-1384 branch.

Things to do/discuss:

  1. DatabaseConfiguration checks should be mainly run before deployment (connected to production DB), so we can check that the production database is properly configured, but this will require defining django-test-migration as the main dependency. Personally, I dislike putting dependencies with test in their names to main ones, what do you think about it? Do you have any other ideas?
  2. We need to release a new version of django-test-migrations, but it's necessary to fix MySQL support - check apply_initial_migration fails  django-test-migrations#122 for more details

@sobolevn
Copy link
Member Author

DatabaseConfiguration checks should be mainly run before deployment (connected to production DB), so we can check that the production database is properly configured, but this will require defining django-test-migration as the main dependency. Personally, I dislike putting dependencies with test in their names to main ones, what do you think about it? Do you have any other ideas?

Currently it lives in development only 🤔
https://github.com/wemake-services/wemake-django-template/blob/master/%7B%7Bcookiecutter.project_name%7D%7D/server/settings/environments/development.py#L33

I guess that adding all django-test-migrations to prod dependencies just because of this one feature is not a good idea for our template.

It might be useful for some people, but in this case it is up to them to decide.
I will add a short doc comment about it.

sobolevn added a commit that referenced this issue Jan 28, 2021
@skarzi
Copy link

skarzi commented Feb 2, 2021

I guess that adding all django-test-migrations to prod dependencies just because of this one feature is not a good idea for our template.

I totally agree. I just started thinking about moving our checks to django-extra-checks and leave django-test-migrations with only one responsibility - migrations testing,
What's your opinion on that?

@sobolevn
Copy link
Member Author

sobolevn commented Feb 2, 2021

Hm, this way we would have django-extra-checks as a main dependency. But, it really has a lot of dev-only checks.
Like:

  • model and fields definition validation
  • admin definition validation
  • etc

So, this is not really a change.

Maybe we can create a new lib? Something like django-pre-deploy-checks?

This way we can clearly indicate that these checks are only meant to be executed before the actual deploy.
Ideas:

  • Static files are there
  • Database is accessible and configured properly
  • Required binaries (like nginx or gunicorn) are there

How do you like it? If you like the idea, I will create some initial boilerplate and start things up!

@skarzi
Copy link

skarzi commented Feb 2, 2021

Maybe we can create a new lib? Something like django-pre-deploy-checks?

That's a great idea, let's give it a go!

@sobolevn
Copy link
Member Author

sobolevn commented Feb 2, 2021

Done! https://github.com/wemake-services/django-pre-deploy-checks

Added you as a maintainer 👍

@skarzi
Copy link

skarzi commented Feb 2, 2021

super 👍
I will try to move all required code from django-test-migrations to the new package on the weekend

@sobolevn sobolevn reopened this Feb 10, 2021
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