Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#28160 closed Bug (fixed)

Prevent hiding GDAL errors if it's not installed

Reported by: Tom Kazimiers Owned by: nobody
Component: GIS Version: 1.10
Severity: Normal Keywords: GIS, GDAL, Contrib, Bug
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tom Kazimiers)

Under some circumstances django.contrib.gis.gdal.HAS_GDAL is False. This can be due to not having GDAL installed in the first place or by overriding one's OS name (and thereby causing the test in to result in an exception which in turns causes HAS_GDAL = False).¹

Now with HAS_GDAL = False, I can't start Django anymore (neither the shell or any other command), because I get the following error:

django.core.exceptions.ImproperlyConfigured: 'postgresql' isn't an available database backend.
Try using 'django.db.backends.XXX', where XXX is one of:
    'mysql', 'oracle', 'postgresql', 'sqlite3'
Error was: No module named postgresql.base

Digging a bit deeper into this reveals this is actually an ImportError: Error was: cannot import name GDALRaster. Using other django.contrib.gis functionality should also work without GDAL (and did so in the past).

The problem seems to be that in commit bbfad84dd980a97174c3b061a3d1b5f1373c380d (here) a new import has been added to django/contrib/gis/db/backends/postgis/, which tries to import GDALRaster: from django.contrib.gis.gdal import GDALRaster. This of course fails with HAS_GDAL = False, because GDALRaster has never been defined when HAS_GDAL was set false due to an (expected) exception in django/django/contrib/gis/gdal/, which is imported before from django.contrib.gis.gdal.raster.source import GDALRaster in django/django/contrib/gis/gdal/

So maybe the import in django/contrib/gis/db/backends/postgis/ and the use of GDALRaster should be conditional and only run if HAS_GDAL = True. I can provide a patch if wanted.

This is a problem in both Django 1.10 and 1.11.

A workaround for this is to run this code before the (postgis) database driver is loaded (e.g. through an own custom database driver that just sub-classes the original):

import django.contrib.gis.gdal
class GDALRasterMock(object):
django.contrib.gis.gdal.GDALRaster = GDALRasterMock

¹ In my project I need to do the latter to disable GDAL support completely, because loading the library conflicts with pgmagick on Ubuntu and there is no other way to disable GDAL, but this shouldn't matter here. It would be nice though to have a way to force HAS_GDAL = False, because unlike said in django/django/contrib/gis/gdal/, setting GDAL_LIBRARY_PATH='/null/path' doesn't work (the automatic OS checks override this).

Change History (10)

comment:1 by Tom Kazimiers, 7 years ago

Description: modified (diff)

Add monkey-patch workaround

comment:2 by Claude Paroz, 7 years ago

Triage Stage: UnreviewedAccepted

In Django 1.11, GDAL is now mandatory in order to use django.contrib.gis (read
However, somewhere in the code, the import exception is swallowed, which isn't nice at all. There should be at least a clear message that GDAL cannot be imported.

comment:3 by Tom Kazimiers, 7 years ago

I see, thanks for the pointer to the Django 1.11 release notes that make clear GDAL is required from 1.11 on to use django.contrib.gis. This makes fixing the HAS_GDAL = False issue maybe not really worth it (given that one can work around it) and I better figure out why using GDAL breaks TIFF support in pgmagick (at least for me on Ubuntu). Fixing the swallowed exception would be nice though.

comment:4 by Tim Graham, 7 years ago

Has patch: set
Patch needs improvement: set
Summary: Can't start with django.contrib.gis.gdal.HAS_GDAL = False due to ImportError of GDALRasterPrevent hiding GDAL errors

I think we can remove HAS_GDAL considering it's used only in tests and instead use to skip gis_tests unless a spatial backend is being tested. Any other ideas or thoughts?

PR (one test still needs fixing)

comment:5 by Tim Graham, 7 years ago

Patch needs improvement: unset
Summary: Prevent hiding GDAL errorsPrevent hiding GDAL errors if it's not installed

comment:6 by Tim Graham <timograham@…>, 7 years ago

In 89053725:

Made run gis_tests only when using a GIS database backend.

This facilitates other changes like refs #28160.

comment:7 by Tim Graham <timograham@…>, 7 years ago

In 444cdf6:

[1.11.x] Made run gis_tests only when using a GIS database backend.

This facilitates other changes like refs #28160.

Backport of 890537253cf235091816d27a5c2fb64943c8e34a from master

comment:8 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: newclosed

In 2dc3280:

Fixed #28160 -- Prevented hiding GDAL exceptions when it's not installed.

comment:9 by Tim Graham <timograham@…>, 7 years ago

In 68ebc24:

[1.11.x] Fixed #28160 -- Prevented hiding GDAL exceptions when it's not installed.

Backport of 2dc3280254ae06ca1fe664abf55749fe12a699d4 from master

comment:10 by GitHub <noreply@…>, 7 years ago

In 4841fafb:

[1.11.x] Refs #28160 -- Skipped a GeoManager test if not using a GIS database backend.

The test errors if GDAL isn't installed.

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