diff --git a/common b/common index 6093983195e..a3297a9314d 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 6093983195ea4699169e7652635e437e603a93c1 +Subproject commit a3297a9314d0b221cd2ce3c429fdf0fc575eefe7 diff --git a/docs/dev/index.rst b/docs/dev/index.rst index 99ebeaca194..4e7f70d6d55 100644 --- a/docs/dev/index.rst +++ b/docs/dev/index.rst @@ -25,6 +25,7 @@ or taking the open source Read the Docs codebase for your own custom installatio style-guide front-end i18n + migrations server-side-search search-integration subscriptions diff --git a/docs/dev/migrations.rst b/docs/dev/migrations.rst new file mode 100644 index 00000000000..3c4117286e4 --- /dev/null +++ b/docs/dev/migrations.rst @@ -0,0 +1,133 @@ +Database migrations +=================== + +We use `Django migrations `__ to manage database schema changes, +and the `django-safemigrate `__ package to ensure that migrations are run in a given order to avoid downtime. + +To make sure that migrations don't cause downtime, +the following rules should be followed for each case. + +Adding a new field +------------------ + +**When adding a new field to a model, it should be nullable.** +This way, the database can be migrated without downtime, and the field can be populated later. +Don't forget to make the field non-nullable in a separate migration after the data has been populated. +You can achieve this by following these steps: + +- #. Set the new field as ``null=True`` and ``blank=True`` in the model. + + .. code-block:: python + + class MyModel(models.Model): + new_field = models.CharField( + max_length=100, null=True, blank=True, default="default" + ) + +- #. Make sure that the field is always populated with a proper value in the new code, + and the code handles the case where the field is null. + + .. code-block:: python + + if my_model.new_field in [None, "default"]: + pass + + + # If it's a boolean field, make sure that the null option is removed from the form. + class MyModelForm(forms.ModelForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields["new_field"].widget = forms.CheckboxInput() + self.fields["new_field"].empty_value = False + +- #. Create the migration file (let's call this migration ``app 0001``), + and mark it as ``Safe.before_deploy``. + + .. code-block:: python + + from django.db import migrations, models + from django_safemigrate import Safe + + + class Migration(migrations.Migration): + safe = Safe.before_deploy + +- #. Create a data migration to populate all null values of the new field with a proper value (let's call this migration ``app 0002``), + and mark it as ``Safe.after_deploy``. + + .. code-block:: python + + from django.db import migrations + + + def migrate(apps, schema_editor): + MyModel = apps.get_model("app", "MyModel") + MyModel.objects.filter(new_field=None).update(new_field="default") + + + class Migration(migrations.Migration): + safe = Safe.after_deploy + + operations = [ + migrations.RunPython(migrate), + ] + +- #. After the deploy has been completed, create a new migration to set the field as non-nullable (let's call this migration ``app 0003``). + Run this migration on a new deploy, you can mark it as ``Safe.before_deploy`` or ``Safe.always``. +- #. Remove any handling of the null case from the code. + +At the end, the deploy should look like this: + +- Deploy web-extra. +- Run ``django-admin safemigrate`` to run the migration ``app 0001``. +- Deploy the webs +- Run ``django-admin migrate`` to run the migration ``app 0002``. +- Create a new migration to set the field as non-nullable, + and apply it on the next deploy. + +Removing a field +---------------- + +**When removing a field from a model, +all usages of the field should be removed from the code before the field is removed from the model, +and the field should be nullable.** +You can achieve this by following these steps: + +- #. Remove all usages of the field from the code. +- #. Set the field as ``null=True`` and ``blank=True`` in the model. + + .. code-block:: python + + class MyModel(models.Model): + field_to_delete = models.CharField(max_length=100, null=True, blank=True) + +- #. Create the migration file (let's call this migration ``app 0001``), + and mark it as ``Safe.before_deploy``. + + .. code-block:: python + + from django.db import migrations, models + from django_safemigrate import Safe + + + class Migration(migrations.Migration): + safe = Safe.before_deploy + +- #. Create a migration to remove the field from the database (let's call this migration ``app 0002``), + and mark it as ``Safe.after_deploy``. + + .. code-block:: python + + from django.db import migrations, models + from django_safemigrate import Safe + + + class Migration(migrations.Migration): + safe = Safe.after_deploy + +At the end, the deploy should look like this: + +- Deploy web-extra. +- Run ``django-admin safemigrate`` to run the migration ``app 0001``. +- Deploy the webs +- Run ``django-admin migrate`` to run the migration ``app 0002``. diff --git a/readthedocs/builds/migrations/0057_migrate_timestamp_fields.py b/readthedocs/builds/migrations/0057_migrate_timestamp_fields.py new file mode 100644 index 00000000000..5192f283882 --- /dev/null +++ b/readthedocs/builds/migrations/0057_migrate_timestamp_fields.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.9 on 2024-02-01 20:29 + +import datetime + +from django.db import migrations +from django_safemigrate import Safe + + +def migrate(apps, schema_editor): + """ + Migrate the created and modified fields of the Version model to have a non-null value. + + This date corresponds to the release date of 5.6.5, + when the created field was added to the Version model + at https://github.com/readthedocs/readthedocs.org/commit/d72ee6e27dc398b97e884ccec8a8cf135134faac. + """ + Version = apps.get_model("builds", "Version") + date = datetime.datetime(2020, 11, 23, tzinfo=datetime.timezone.utc) + Version.objects.filter(created=None).update(created=date) + Version.objects.filter(modified=None).update(modified=date) + + +class Migration(migrations.Migration): + safe = Safe.before_deploy + + dependencies = [ + ("builds", "0056_alter_versionautomationrule_priority"), + ] + + operations = [ + migrations.RunPython(migrate), + ] diff --git a/readthedocs/builds/migrations/0058_alter_version_created_alter_version_modified.py b/readthedocs/builds/migrations/0058_alter_version_created_alter_version_modified.py new file mode 100644 index 00000000000..f66dea80bbe --- /dev/null +++ b/readthedocs/builds/migrations/0058_alter_version_created_alter_version_modified.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.9 on 2024-02-01 20:38 + +import django.utils.timezone +import django_extensions.db.fields +from django.db import migrations +from django_safemigrate import Safe + + +class Migration(migrations.Migration): + safe = Safe.after_deploy + + dependencies = [ + ("builds", "0057_migrate_timestamp_fields"), + ] + + operations = [ + migrations.AlterField( + model_name="version", + name="created", + field=django_extensions.db.fields.CreationDateTimeField( + auto_now_add=True, + default=django.utils.timezone.now, + verbose_name="created", + ), + preserve_default=False, + ), + migrations.AlterField( + model_name="version", + name="modified", + field=django_extensions.db.fields.ModificationDateTimeField( + auto_now=True, + default=django.utils.timezone.now, + verbose_name="modified", + ), + preserve_default=False, + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 926984456f7..5947c9d7a0d 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -13,7 +13,6 @@ from django.utils import timezone from django.utils.translation import gettext from django.utils.translation import gettext_lazy as _ -from django_extensions.db.fields import CreationDateTimeField, ModificationDateTimeField from django_extensions.db.models import TimeStampedModel from polymorphic.models import PolymorphicModel @@ -92,19 +91,6 @@ class Version(TimeStampedModel): """Version of a ``Project``.""" - # Overridden from TimeStampedModel just to allow null values. - # TODO: remove after deploy. - created = CreationDateTimeField( - _('created'), - null=True, - blank=True, - ) - modified = ModificationDateTimeField( - _('modified'), - null=True, - blank=True, - ) - project = models.ForeignKey( Project, verbose_name=_('Project'), diff --git a/readthedocs/integrations/migrations/0013_set_timestamp_fields_as_no_null.py b/readthedocs/integrations/migrations/0013_set_timestamp_fields_as_no_null.py new file mode 100644 index 00000000000..81113f6a7bc --- /dev/null +++ b/readthedocs/integrations/migrations/0013_set_timestamp_fields_as_no_null.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.9 on 2024-02-01 20:10 + +import django.utils.timezone +import django_extensions.db.fields +from django.db import migrations +from django_safemigrate import Safe + + +class Migration(migrations.Migration): + safe = Safe.always + + dependencies = [ + ("integrations", "0012_migrate_timestamp_fields"), + ] + + operations = [ + migrations.AlterField( + model_name="integration", + name="created", + field=django_extensions.db.fields.CreationDateTimeField( + auto_now_add=True, + default=django.utils.timezone.now, + verbose_name="created", + ), + preserve_default=False, + ), + migrations.AlterField( + model_name="integration", + name="modified", + field=django_extensions.db.fields.ModificationDateTimeField( + auto_now=True, + default=django.utils.timezone.now, + verbose_name="modified", + ), + preserve_default=False, + ), + ] diff --git a/readthedocs/integrations/models.py b/readthedocs/integrations/models.py index ea812aee9fd..06a1d153f73 100644 --- a/readthedocs/integrations/models.py +++ b/readthedocs/integrations/models.py @@ -9,7 +9,6 @@ from django.utils.crypto import get_random_string from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ -from django_extensions.db.fields import CreationDateTimeField, ModificationDateTimeField from django_extensions.db.models import TimeStampedModel from pygments import highlight from pygments.formatters import HtmlFormatter @@ -278,19 +277,6 @@ class Integration(TimeStampedModel): INTEGRATIONS = WEBHOOK_INTEGRATIONS - # Overridden from TimeStampedModel just to allow null values. - # TODO: remove after deploy. - created = CreationDateTimeField( - _("created"), - null=True, - blank=True, - ) - modified = ModificationDateTimeField( - _("modified"), - null=True, - blank=True, - ) - project = models.ForeignKey( Project, related_name="integrations", diff --git a/readthedocs/projects/fixtures/test_data.json b/readthedocs/projects/fixtures/test_data.json index c3f28045f75..cfc3284f9d6 100644 --- a/readthedocs/projects/fixtures/test_data.json +++ b/readthedocs/projects/fixtures/test_data.json @@ -888,8 +888,8 @@ "model": "builds.version", "pk": 1, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 1, "type": "unknown", "identifier": "2ff3d36340fa4d3d39424e8464864ca37c5f191c", @@ -912,8 +912,8 @@ "model": "builds.version", "pk": 2, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 1, "type": "unknown", "identifier": "354456a7dba2a75888e2fe91f6d921e5fe492bcd", @@ -936,8 +936,8 @@ "model": "builds.version", "pk": 3, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 1, "type": "unknown", "identifier": "master", @@ -960,8 +960,8 @@ "model": "builds.version", "pk": 4, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 1, "type": "unknown", "identifier": "not_ok", @@ -984,8 +984,8 @@ "model": "builds.version", "pk": 8, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 1, "type": "unknown", "identifier": "awesome", @@ -1008,8 +1008,8 @@ "model": "builds.version", "pk": 18, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 6, "type": "tag", "identifier": "2404a34eba4ee9c48cc8bc4055b99a48354f4950", @@ -1032,8 +1032,8 @@ "model": "builds.version", "pk": 19, "fields": { - "created": null, - "modified": null, + "created": "2022-02-22T10:55:46.784Z", + "modified": "2022-02-22T10:55:46.784Z", "project": 6, "type": "tag", "identifier": "f55c28e560c92cafb6e6451f8084232b6d717603", diff --git a/readthedocs/projects/migrations/0114_set_timestamp_fields_as_no_null.py b/readthedocs/projects/migrations/0114_set_timestamp_fields_as_no_null.py new file mode 100644 index 00000000000..ce4b1c5b426 --- /dev/null +++ b/readthedocs/projects/migrations/0114_set_timestamp_fields_as_no_null.py @@ -0,0 +1,33 @@ +# Generated by Django 4.2.9 on 2024-02-01 20:10 + +import django.utils.timezone +from django.db import migrations, models +from django_safemigrate import Safe + + +class Migration(migrations.Migration): + safe = Safe.always + + dependencies = [ + ("projects", "0113_disable_analytics_addons"), + ] + + operations = [ + migrations.AlterField( + model_name="domain", + name="skip_validation", + field=models.BooleanField( + default=False, verbose_name="Skip validation process." + ), + ), + migrations.AlterField( + model_name="domain", + name="validation_process_start", + field=models.DateTimeField( + auto_now_add=True, + default=django.utils.timezone.now, + verbose_name="Start date of the validation process.", + ), + preserve_default=False, + ), + ] diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 82ded4eb40f..f4f708ee31f 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1830,14 +1830,10 @@ class Domain(TimeStampedModel): skip_validation = models.BooleanField( _("Skip validation process."), default=False, - # TODO: remove after deploy. - null=True, ) validation_process_start = models.DateTimeField( _("Start date of the validation process."), auto_now_add=True, - # TODO: remove after deploy. - null=True, ) # Strict-Transport-Security header options diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 453ad881ec4..37bd7a1a631 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -237,6 +237,7 @@ def INSTALLED_APPS(self): # noqa 'simple_history', 'djstripe', 'django_celery_beat', + "django_safemigrate.apps.SafeMigrateConfig", # our apps 'readthedocs.projects', diff --git a/requirements/deploy.txt b/requirements/deploy.txt index 3c4eb9bb3e2..575c02e9000 100644 --- a/requirements/deploy.txt +++ b/requirements/deploy.txt @@ -110,6 +110,7 @@ django==4.2.9 # django-filter # django-formtools # django-polymorphic + # django-safemigrate # django-storages # django-structlog # django-taggit @@ -150,12 +151,12 @@ django-ipware==5.0.2 # django-structlog django-polymorphic==3.1.0 # via -r requirements/pip.txt +django-safemigrate==4.2 + # via -r requirements/pip.txt django-simple-history==3.0.0 # via -r requirements/pip.txt django-storages[boto3]==1.14.2 - # via - # -r requirements/pip.txt - # django-storages + # via -r requirements/pip.txt django-structlog==2.2.0 # via -r requirements/pip.txt django-taggit==5.0.1 @@ -286,7 +287,6 @@ pyjwt[crypto]==2.8.0 # via # -r requirements/pip.txt # django-allauth - # pyjwt pyquery==2.0.0 # via -r requirements/pip.txt python-crontab==3.0.0 diff --git a/requirements/docker.txt b/requirements/docker.txt index 96bf16c737b..f2164d22623 100644 --- a/requirements/docker.txt +++ b/requirements/docker.txt @@ -121,6 +121,7 @@ django==4.2.9 # django-filter # django-formtools # django-polymorphic + # django-safemigrate # django-storages # django-structlog # django-taggit @@ -161,12 +162,12 @@ django-ipware==5.0.2 # django-structlog django-polymorphic==3.1.0 # via -r requirements/pip.txt +django-safemigrate==4.2 + # via -r requirements/pip.txt django-simple-history==3.0.0 # via -r requirements/pip.txt django-storages[boto3]==1.14.2 - # via - # -r requirements/pip.txt - # django-storages + # via -r requirements/pip.txt django-structlog==2.2.0 # via -r requirements/pip.txt django-taggit==5.0.1 @@ -315,7 +316,6 @@ pyjwt[crypto]==2.8.0 # via # -r requirements/pip.txt # django-allauth - # pyjwt pyproject-api==1.6.1 # via tox pyquery==2.0.0 diff --git a/requirements/docs.txt b/requirements/docs.txt index 318af4135bf..c22ccf4ce8c 100644 --- a/requirements/docs.txt +++ b/requirements/docs.txt @@ -85,7 +85,7 @@ sphinx==7.2.6 # sphinxcontrib-jquery # sphinxemoji # sphinxext-opengraph -sphinx-autobuild==2021.3.14 +sphinx-autobuild==2024.2.4 # via -r requirements/docs.in sphinx-copybutton==0.5.2 # via -r requirements/docs.in diff --git a/requirements/pip.in b/requirements/pip.in index 26026dc12d0..01ea60f618c 100644 --- a/requirements/pip.in +++ b/requirements/pip.in @@ -36,6 +36,8 @@ django-vanilla-views # dependency as well. jsonfield +django-safemigrate + requests requests-toolbelt slumber diff --git a/requirements/pip.txt b/requirements/pip.txt index e411525de71..c856705fca4 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -73,6 +73,7 @@ django==4.2.9 # django-filter # django-formtools # django-polymorphic + # django-safemigrate # django-storages # django-structlog # django-taggit @@ -113,6 +114,8 @@ django-ipware==5.0.2 # django-structlog django-polymorphic==3.1.0 # via -r requirements/pip.in +django-safemigrate==4.2 + # via -r requirements/pip.in django-simple-history==3.0.0 # via -r requirements/pip.in django-storages[boto3]==1.14.2 @@ -198,9 +201,7 @@ pycparser==2.21 pygments==2.17.2 # via -r requirements/pip.in pyjwt[crypto]==2.8.0 - # via - # django-allauth - # pyjwt + # via django-allauth pyquery==2.0.0 # via -r requirements/pip.in python-crontab==3.0.0 diff --git a/requirements/testing.txt b/requirements/testing.txt index 0671d1897aa..eb526ce7221 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -71,9 +71,7 @@ click-repl==0.3.0 # -r requirements/pip.txt # celery coverage[toml]==7.4.1 - # via - # coverage - # pytest-cov + # via pytest-cov cron-descriptor==1.4.3 # via # -r requirements/pip.txt @@ -113,6 +111,7 @@ django==4.2.9 # django-filter # django-formtools # django-polymorphic + # django-safemigrate # django-storages # django-structlog # django-taggit @@ -155,12 +154,12 @@ django-ipware==5.0.2 # django-structlog django-polymorphic==3.1.0 # via -r requirements/pip.txt +django-safemigrate==4.2 + # via -r requirements/pip.txt django-simple-history==3.0.0 # via -r requirements/pip.txt django-storages[boto3]==1.14.2 - # via - # -r requirements/pip.txt - # django-storages + # via -r requirements/pip.txt django-structlog==2.2.0 # via -r requirements/pip.txt django-taggit==5.0.1 @@ -288,7 +287,6 @@ pyjwt[crypto]==2.8.0 # via # -r requirements/pip.txt # django-allauth - # pyjwt pyquery==2.0.0 # via -r requirements/pip.txt pytest==8.0.0