wemake-django-template icon indicating copy to clipboard operation
wemake-django-template copied to clipboard

Enable `django_checks` check from `django-test-migrations`

Open sobolevn opened this issue 4 years ago • 9 comments

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?

sobolevn avatar Nov 26 '20 11:11 sobolevn

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

skarzi avatar Nov 27 '20 10:11 skarzi

Awesome, thanks!

sobolevn avatar Nov 27 '20 11:11 sobolevn

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 https://github.com/wemake-services/django-test-migrations/issues/122 for more details

skarzi avatar Nov 28 '20 18:11 skarzi

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 avatar Jan 28 '21 11:01 sobolevn

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?

skarzi avatar Feb 02 '21 09:02 skarzi

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!

sobolevn avatar Feb 02 '21 10:02 sobolevn

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

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

skarzi avatar Feb 02 '21 10:02 skarzi

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

Added you as a maintainer 👍

sobolevn avatar Feb 02 '21 12:02 sobolevn

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

skarzi avatar Feb 02 '21 14:02 skarzi