geodesign / django-raster

Django-raster allows you to create tiled map services (TMS) and raster map algebra end points for web maps. It is Python-based, and requires GeoDjango with a PostGIS backend.
BSD 3-Clause "New" or "Revised" License
96 stars 39 forks source link

django-raster incompatible with celery >= 4.* #16

Closed lsix closed 7 years ago

lsix commented 7 years ago

Django-raster is not compatible with upstream version of celery.

The changelog of the 4.0 release states:

I am planning to drop the use of celery to parallelize import of tiles as follows (this passes all the test suite locally) since I do the map imports in an async celery task anyway.

Would a PR with such change make it to master?

diff --git a/raster/tiles/parser.py b/raster/tiles/parser.py
index 41c4c16..35e27e1 100644
--- a/raster/tiles/parser.py
+++ b/raster/tiles/parser.py
@@ -8,7 +8,6 @@ import zipfile

 import numpy
 from celery import current_app, group
-from celery.contrib.methods import task_method

 from django.conf import settings
 from django.contrib.gis.gdal import GDALRaster, OGRGeometry
@@ -290,9 +289,8 @@ class RasterLayerParser(object):

         self.log('Creating {0} tiles in {1} quadrants at zoom {2}.'.format(self.nr_of_tiles(zoom), len(quadrants), zoom))

-        # Process quadrants in parallell
-        quadrant_task_group = group(self.process_quadrant.si(indexrange, zoom) for indexrange in quadrants)
-        quadrant_task_group.apply()
+        for indexrange in quadrants:
+            self.process_quadrant(indexrange, zoom)

         # Store histogram data
         if zoom == self.max_zoom:
@@ -305,7 +303,6 @@ class RasterLayerParser(object):

     _quadrant_count = 0

-    @current_app.task(filter=task_method)
     def process_quadrant(self, indexrange, zoom):
         """
         Create raster tiles for a quadrant of tiles defined by a x-y-z index
yellowcap commented 7 years ago

Interesting, thanks for pointing that out. Ideally the process_quadrant method would be turned into a standalone task, so that the quadrant parsing would continue to be parallelized. However, that would require some larger restructuring of the parser. I think your solution works until that happens, so yes if you open a pull request, I'll merge it in.

lsix commented 7 years ago

Yes, my initial plan was to make the method a separate task, but I’ll need to get into the codebase first to do it properly (hopefully, I’ll have time soon).

I have openned the PR #17 to include the “quick-fix” until a proper replacement is implemented.

yellowcap commented 7 years ago

Solved with https://github.com/geodesign/django-raster/commit/948864d97b789b0d12e505ed50816a0f76b92d2f thanks @Isix