Code

Opened 6 months ago

Closed 6 months ago

#21283 closed Bug (fixed)

Migrations created in wrong directory for models defined in model package

Reported by: anonymous Owned by: MarkusH
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: info@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django supports to split a bunch of models into a separate Python modules which are placed in a models Python package. However the django.db.migrations.writer.MigrationWriter does not reflect this fact in its path property.

The solution is easy:

If myapp.models is a Python app_module (currently line 61) has a __path__ attribute. If it is a Python module there is no such attribute.

I'm going to write a patch and open a pull request for further discussion.

Attachments (0)

Change History (8)

comment:1 Changed 6 months ago by timo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 months ago by Markus Holtermann <info@…>

  • Has patch set
  • Needs tests set
  • Owner changed from MarkusH to anonymous
  • Status changed from new to assigned

The pull request is at https://github.com/django/django/pull/1763

Right know I don't have a clue how to write a test case for this issue.

comment:3 Changed 6 months ago by MarkusH

  • Cc info@… added
  • Owner changed from anonymous to MarkusH

comment:4 Changed 6 months ago by timo

Could you make a separate test app like "migrations_model_package" and make sure migrations are properly generated there?

comment:5 Changed 6 months ago by loic84

@timo, shouldn't this use the app cache methods that you reworked at some point to better deal with models in exotic locations?

As for testing, dunno if this is going to play nice with MIGRATION_MODULES that we need to use in tests.

comment:6 Changed 6 months ago by timo

That would be get_app_path() introduced in 31c13a99bb9ebdaf12ccab4e880c5da930d86e79, I think.

comment:7 Changed 6 months ago by loic84

I had a go at this https://github.com/django/django/pull/1772, feedback welcome.

comment:8 Changed 6 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 584110417f6a3c16cc0c6723825a21940a9a2bc4:

Fixed #21283 -- Added support for migrations if models is a package.

Thanks Markus Holtermann for the report.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.