Opened 3 years ago

Closed 2 years ago

#22645 closed Bug (fixed)

Gis tests broken without a models.py

Reported by: Florian Apolloner Owned by: Andrew Godwin
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: Aymeric Augustin, Andrew Godwin Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This commit: https://github.com/django/django/commit/cebd8753c65ec33730f495bb7085f34884f7c3d6 broke the GIS tests with the following error: http://ci.djangoproject.com/job/Django/database=postgis,python=python2.7/4456/testReport/junit/django.contrib.gis.tests.gis_migrations.test_commands/MigrateTests/test_migrate_gis_2/ -- readding an empty models.py fixed it but should be considered as workaround.

Assigning to migrations, but could also be an app-refactor issue?

Change History (13)

comment:1 Changed 3 years ago by Andrew Godwin

Owner: changed from nobody to Andrew Godwin
Status: newassigned

I'll look into this. Probably a simple fix.

comment:2 Changed 3 years ago by Aymeric Augustin

IIRC SpatialSysRef is a model, so it should be imported in models.py.

comment:3 Changed 3 years ago by Andrew Godwin <andrew@…>

Resolution: fixed
Status: assignedclosed

In a4737bf6ae36a5f1cb29f2232f6deeff084fabff:

Fixed #22645: Allow apps with no models module to still have migrations

comment:4 Changed 3 years ago by Andrew Godwin

It's fine, this was just an overzealous check in MigrationLoader that I've removed.

comment:5 Changed 3 years ago by Andrew Godwin <andrew@…>

In 12c72796c3a385f21bc97389bef91dcd4148c0f9:

Revert "Fixed #22645: Allow apps with no models module to still have migrations"

This reverts commit a4737bf6ae36a5f1cb29f2232f6deeff084fabff.

comment:6 Changed 3 years ago by Tim Graham

Resolution: fixed
Status: closednew

comment:7 Changed 2 years ago by Claude Paroz

Should we then simply change the comment? Removing These aliases are provided for backwards-compatibility.?

comment:8 Changed 2 years ago by Claude Paroz

Even if the problem related to migrations for apps without models.py will be solved by #23618, I still think that we should keep those imports, as Aymeric said in comment:2, and update the comment accordingly. We have now the has_spatialrefsys_table database feature for the conditional import.

comment:9 Changed 2 years ago by Tim Graham

Has patch: set

PR to restore the spatial models in models.py.

comment:10 Changed 2 years ago by Claude Paroz

I'm sorry, but I'm not so sure any more about recreating models.py. The problem is that the presence of SpatialRefSys/GeometryColumns is backend dependent, so adding them based on the default connection object might break in case of multi-db setups where a non-default database has no such tables, like MySQL.
I've just tested removing the empty models.py from django.contrib.gis and the test suite now passes: https://github.com/django/django/pull/3724

comment:11 Changed 2 years ago by Tim Graham

No objection from me.

comment:12 Changed 2 years ago by Claude Paroz <claude@…>

In a5499b0916298caedfd683041515f4e3aa2c5d6d:

Removed empty gis models.py

Refs #22645.

comment:13 Changed 2 years ago by Claude Paroz

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top