From adf40e1d5679733a6609da0d16408613ed66cf2d Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Fri, 17 Sep 2021 10:01:23 +0200 Subject: [PATCH] Refactor our migrate command monkeypatching --- .../base/management/commands/_migrations.py | 46 ++++++++++++++++++ .../management/commands/makemigrations.py | 47 +------------------ .../base/management/commands/migrate.py | 19 ++++---- 3 files changed, 59 insertions(+), 53 deletions(-) create mode 100644 src/pretix/base/management/commands/_migrations.py diff --git a/src/pretix/base/management/commands/_migrations.py b/src/pretix/base/management/commands/_migrations.py new file mode 100644 index 0000000000..e35f8c00ac --- /dev/null +++ b/src/pretix/base/management/commands/_migrations.py @@ -0,0 +1,46 @@ +""" +Django, for theoretically very valid reasons, creates migrations for *every single thing* +we change on a model. Even the `help_text`! This makes sense, as we don't know if any +database backend unknown to us might actually use this information for its database schema. + +However, pretix only supports PostgreSQL, MySQL, MariaDB and SQLite and we can be pretty +certain that some changes to models will never require a change to the database. In this case, +not creating a migration for certain changes will save us some performance while applying them +*and* allow for a cleaner git history. Win-win! + +Only caveat is that we need to do some dirty monkeypatching to achieve it... +""" +from django.db import models +from django.db.migrations.operations import models as modelops +from django_countries.fields import CountryField + + +def monkeypatch_migrations(): + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("verbose_name") + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("verbose_name_plural") + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("ordering") + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("get_latest_by") + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("default_manager_name") + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("permissions") + modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("default_permissions") + IGNORED_ATTRS = [ + # (field type, attribute name, banlist of field sub-types) + (models.Field, 'verbose_name', []), + (models.Field, 'help_text', []), + (models.Field, 'validators', []), + (models.Field, 'editable', [models.DateField, models.DateTimeField, models.DateField, models.BinaryField]), + (models.Field, 'blank', [models.DateField, models.DateTimeField, models.AutoField, models.NullBooleanField, + models.TimeField]), + (models.CharField, 'choices', [CountryField]) + ] + + original_deconstruct = models.Field.deconstruct + + def new_deconstruct(self): + name, path, args, kwargs = original_deconstruct(self) + for ftype, attr, banlist in IGNORED_ATTRS: + if isinstance(self, ftype) and not any(isinstance(self, ft) for ft in banlist): + kwargs.pop(attr, None) + return name, path, args, kwargs + + models.Field.deconstruct = new_deconstruct diff --git a/src/pretix/base/management/commands/makemigrations.py b/src/pretix/base/management/commands/makemigrations.py index 9ed7c8afb2..b3c5b1adf7 100644 --- a/src/pretix/base/management/commands/makemigrations.py +++ b/src/pretix/base/management/commands/makemigrations.py @@ -32,53 +32,10 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. -""" -Django, for theoretically very valid reasons, creates migrations for *every single thing* -we change on a model. Even the `help_text`! This makes sense, as we don't know if any -database backend unknown to us might actually use this information for its database schema. - -However, pretix only supports PostgreSQL, MySQL, MariaDB and SQLite and we can be pretty -certain that some changes to models will never require a change to the database. In this case, -not creating a migration for certain changes will save us some performance while applying them -*and* allow for a cleaner git history. Win-win! - -Only caveat is that we need to do some dirty monkeypatching to achieve it... -""" from django.core.management.commands.makemigrations import Command as Parent -from django.db import models -from django.db.migrations.operations import models as modelops -from django_countries.fields import CountryField +from ._migrations import monkeypatch_migrations -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("verbose_name") -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("verbose_name_plural") -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("ordering") -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("get_latest_by") -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("default_manager_name") -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("permissions") -modelops.AlterModelOptions.ALTER_OPTION_KEYS.remove("default_permissions") -IGNORED_ATTRS = [ - # (field type, attribute name, banlist of field sub-types) - (models.Field, 'verbose_name', []), - (models.Field, 'help_text', []), - (models.Field, 'validators', []), - (models.Field, 'editable', [models.DateField, models.DateTimeField, models.DateField, models.BinaryField]), - (models.Field, 'blank', [models.DateField, models.DateTimeField, models.AutoField, models.NullBooleanField, - models.TimeField]), - (models.CharField, 'choices', [CountryField]) -] - -original_deconstruct = models.Field.deconstruct - - -def new_deconstruct(self): - name, path, args, kwargs = original_deconstruct(self) - for ftype, attr, banlist in IGNORED_ATTRS: - if isinstance(self, ftype) and not any(isinstance(self, ft) for ft in banlist): - kwargs.pop(attr, None) - return name, path, args, kwargs - - -models.Field.deconstruct = new_deconstruct +monkeypatch_migrations() class Command(Parent): diff --git a/src/pretix/base/management/commands/migrate.py b/src/pretix/base/management/commands/migrate.py index 2a2d803137..ac5dbe4d39 100644 --- a/src/pretix/base/management/commands/migrate.py +++ b/src/pretix/base/management/commands/migrate.py @@ -32,22 +32,25 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. -""" -Django tries to be helpful by suggesting to run "makemigrations" in red font on every "migrate" -run when there are things we have no migrations for. Usually, this is intended, and running -"makemigrations" can really screw up the environment of a user, so we want to prevent novice -users from doing that by going really dirty and filtering it from the output. -""" import sys from django.core.management.base import OutputWrapper from django.core.management.commands.migrate import Command as Parent +from ._migrations import monkeypatch_migrations + +monkeypatch_migrations() class OutputFilter(OutputWrapper): + """ + Django tries to be helpful by suggesting to run "makemigrations" in red font on every "migrate" + run when there are things we have no migrations for. Usually, this is intended, and running + "makemigrations" can really screw up the environment of a user, so we want to prevent novice + users from doing that by going really dirty and filtering it from the output. + """ banlist = ( - "Your models have changes that are not yet reflected", - "Run 'manage.py makemigrations' to make new " + "have changes that are not yet reflected", + "re-run 'manage.py migrate'" ) def write(self, msg, style_func=None, ending=None):