Opened 2 years ago

Closed 2 years ago

#34050 closed Bug (fixed)

Generated migration file is not detected by django because of the name of newly generated migration file

Reported by: Bishal Gautam Owned by: Adam Johnson
Component: Migrations Version: 4.1
Severity: Normal Keywords: migrations
Cc: jacobtylerwalls@…, Adam Johnson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

After a new project is created we run:
python manage.py migrate to sync with our auth database models.

Lets create an app called myapp, register this app on settings. Now we create a model inside myapp as:

class MyModel(models.Model):
    name = models.CharField(max_length=100)
    age = models.IntegerField()

Lets run makemigrations and migrate command for the app. This will generate a migration file with name 0001_initial.py inside myapp/migrations/ folder.

Now let us create a constraints for the model as:

class MyModel(models.Model):
    name = models.CharField(max_length=100)
    age = models.IntegerField()

    class Meta:
        constraints = [
            models.CheckConstraint(
                check=models.Q(age__gte=1),
                name="Age should not be.less.than.one."
            )
        ]

When we added CheckConstraint for our previous model, we need to run makemigrations and migrate to reflect the new changes on database.
Running makemigrations will generate a migration file inside user/migrations/ with name 0002_mymodel_age should not be.less.than.one..py. When we try to run migrate, django will not be able to detect this file and NO changes will be applied to existing database. If we run makemigrations again, the same file will be generated again. No matter how many time we run makemigrations, the same file will be generated again and again. The newly generated migration file is not detected by django migrate and showmigrations command. This is because of the name of migration file. The new migration file name contains serveral dots and because of this name, migrate/showmigration is not able to find the file.

There are mutiple solutions for this:

  • First: Renaming the generated migration file. Remove . (dot) (excluding the .py)from the newly generated migration file. This will make sure that the file is recognized as python file by django migrate/showmigrations command.
  • Second: We can change the name of our constraint name inside the migration as :
class Meta:
        constraints = [
            models.CheckConstraint(
                check=models.Q(age__gte=1),
                name="Age should not be less than one"
            )
        ]

The only difference between the constraint that was above and the constraint that is implemented now is in the value of name parameter for models.Q(). We removed the exisiting "dot" . from the name. Running makemigration will generate a migration file with name 0002_mymodel_age should not be less than one.py. Now running the migrate/showmigrations command will find the new migration file. This example is also commited in the repositoryhttps://github.com/bisalgt/test-django-migrations. One can look at the commit history and checkout to proper stage as needed , described by commit message on the repository commit history.

Using third approach makes sure that no one has to go through the step First and Second as described above. The solution would work out of the box.

Attachments (1)

invalid migration file name generated-unable to create migrations file.png (145.4 KB ) - added by Bishal Gautam 2 years ago.
Invalid character in constraint name leads to invalid file so django is unable to create new migrations file

Download all attachments as: .zip

Change History (13)

comment:1 by David Sanders, 2 years ago

Has patch: set
Needs tests: set
Owner: changed from nobody to Bishal Gautam
Status: newassigned

Can confirm this.

It's worth highlighting is that migrations literally copies the constraint name verbatim for any invalid Python module character not just dots "." and will happily apply these (as long as there's no dots) – if we're not doing any imports of migrations we should be fine.

The PR replaces the dots with underscores - that's one solution which addresses the immediate issue but I'm just wondering whether:

a.) Any invalid character is replaced to ensure there are no more problems like this; or
b.) Migrations (ie showmigrations and migrate) do not skip files with multiple dots?

Last edited 2 years ago by David Sanders (previous) (diff)

by Bishal Gautam, 2 years ago

Invalid character in constraint name leads to invalid file so django is unable to create new migrations file

comment:2 by Bishal Gautam, 2 years ago

Yes, as an experiment, I changed the name of constraint by putting * in the constraint name. Django created migration file name with the exact name of the constraint but failed to create a migration file because of invalid name of the file. I have also added an attachment for this. Kindly find above.

  • This didnot create a migration file and generated a error from django core. I think it still tells the user that the invalid file name causes this issue.
  • In case of dot . , migration file was created everytime. Generated migration file was getting replaced by new one. Django was not able to find the migration file using migrate command. So I think this dot character error is a bit misleading to users and needs to be replaced.

Or may be we can replace most of the frequently used invalid character if replacing doesnot compromise the performance of project.

Last edited 2 years ago by Bishal Gautam (previous) (diff)

comment:3 by David Sanders, 2 years ago

Nice detective work Bishal :)

It's probably worth waiting for a member of the triage team to make a decision on whether to replace all non-valid Python identifier chars or special filesystem chars (or maybe even decide to do something else)

comment:4 by Bishal Gautam, 2 years ago

Thank you so much David.

Looking forward to hear from the triage team.

Regards,
Bishal Gautam

comment:5 by Claude Paroz, 2 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

I think replacing any invalid character is the way to go.

comment:6 by Mariusz Felisiak, 2 years ago

Cc: Adam Johnson added

comment:7 by Adam Johnson, 2 years ago

I’ve made an alternative PR: https://github.com/django/django/pull/16117

comment:8 by Bishal Gautam, 2 years ago

Looks great Adam. Thank you Team for acknowledging this Ticket. Looking forward to contribute more in future.

Regards,
Bishal Gautam

comment:9 by Claude Paroz, 2 years ago

Needs tests: unset
Patch needs improvement: unset

comment:10 by Mariusz Felisiak, 2 years ago

Owner: changed from Bishal Gautam to Adam Johnson

comment:11 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In cd03e8e:

Fixed #34050 -- Replaced invalid chars in migration names with '_'.

Thanks to Bishal Gautam for the report and initial implementation.

Regression in fa58450a9ab8a1bdd2a5090b51b00078fd85ffa6.

Co-Authored-By: Bishal Gautam <bisalgt@…>

Note: See TracTickets for help on using tickets.
Back to Top