combo/apps/dataviz/forms.py

@@ -30,6 +30,7 @@ from combo.utils import cache_during_request, requests, spooler
 
 from .fields import StaticField
 from .models import ChartCell, ChartFiltersCell, ChartNgCell
+from .utils import update_available_statistics
 from .widgets import MultiSelectWidget
 
 
@@ -55,7 +56,7 @@ class ChartForm(forms.ModelForm):
 
 @cache_during_request
 def trigger_statistics_list_refresh():
-    transaction.on_commit(spooler.refresh_statistics_list)
+    transaction.on_commit(lambda: spooler.run(update_available_statistics))
 
 
 class ChartFiltersMixin:

combo/apps/dataviz/models.py

@@ -334,12 +334,20 @@ class ChartNgCell(CellBase):
             invalidate_cache=invalidate_cache,
         )
 
+    def refresh_statistics_data(self, filter_params=None):
+        filter_params = filter_params or self.get_filter_params()
+        try:
+            self.get_statistic_data(invalidate_cache=True, filter_params=filter_params)
+        except MissingVariable:
+            return
+
+        if self.statistic.service_slug != 'bijoe':
+            self.update_subfilters(filter_params)
+
     def get_chart(self, width=None, height=None, raise_if_not_cached=False):
-        filter_params = self.get_filter_params()
-        transaction.on_commit(
-            lambda: spooler.refresh_statistics_data(cell_pk=self.pk, filter_params=filter_params)
-        )
-        response = self.get_statistic_data(filter_params, raise_if_not_cached)
+        transaction.on_commit(lambda: spooler.run(self.refresh_statistics_data))
+
+        response = self.get_statistic_data(self.get_filter_params(), raise_if_not_cached)
         response.raise_for_status()
         response = response.json()
 
@@ -762,6 +770,15 @@ class ChartNgCell(CellBase):
     def get_cache_key(self, filters_cell_id):
         return 'dataviz:%s:%s' % (filters_cell_id, self.pk)
 
+    def __getstate__(self):
+        # spooler needs to pickle ChartNgCell, but storing an HttpRequest in
+        # self._request is not picklable (not that it was a really good idea to
+        # do that)
+        state = super().__getstate__()
+        if '_request' in state:
+            del state['_request']
+        return state
+
 
 @register_cell_class
 class ChartFiltersCell(CellBase):

combo/settings.py

@@ -387,6 +387,8 @@ CHART_FILTERS_CELL_ENABLED = True
 # default country code for phonenumbers' user phone parsing
 DEFAULT_COUNTRY_CODE = '33'
 
+USE_NEW_SPOOLER = True
+
 
 def debug_show_toolbar(request):
     from debug_toolbar.middleware import show_toolbar as dt_show_toolbar  # pylint: disable=import-error

combo/utils/spooler.py

@@ -14,98 +14,14 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import contextlib
-import logging
-import sys
-from functools import wraps
+try:
+    from hobo.multitenant.spooler import run  # pylint: disable=unused-import
+except ImportError:
+    import pickle
 
-from django.db import close_old_connections, connection
-
-USE_UWSGI = 'uwsgi' in sys.modules
-
-
-logger = logging.getLogger(__name__)
-
-
-def ensure_db(func):
-    """Emulate Django"s setup/teardown of database connections before/after
-    each request"""
-
-    @wraps(func)
-    def f(*args, **kwargs):
-        close_old_connections()
+    def run(func, *args, **kwargs):
         try:
-            return func(*args, **kwargs)
-        finally:
-            close_old_connections()
-
-    return f
-
-
-@contextlib.contextmanager
-def tenant_context(domain):
-    from hobo.multitenant.middleware import TenantMiddleware  # pylint: disable=import-error
-    from tenant_schemas.utils import tenant_context  # pylint: disable=import-error
-
-    tenant = TenantMiddleware.get_tenant_by_hostname(domain)
-    with tenant_context(tenant):
-        yield
-
-
-def tenantspool(func):
-    """Wrap a function with uwsgidecorators.spool storing and restoring the
-    current tenant."""
-    if not USE_UWSGI:
-        return func
-
-    from uwsgidecorators import spool
-
-    @ensure_db
-    @wraps(func)
-    def spooler_func(*args, **kwargs):
-        with contextlib.ExitStack() as stack:
-            if 'domain' in kwargs:
-                stack.enter_context(tenant_context(kwargs.pop('domain')))
-            try:
-                func(*args, **kwargs)
-            except Exception:
-                logger.exception('spooler: exception during %s(%s, %s)', func.__name__, args, kwargs)
-            else:
-                logger.info('spooler: success of %s(%s, %s)', func.__name__, args, kwargs)
-
-    # pass arguments as pickles
-    base_spooler = spool(pass_arguments=True)(spooler_func)
-
-    @wraps(func)
-    def spooler(*args, **kwargs):
-        domain = getattr(getattr(connection, 'tenant', None), 'domain_url', None)
-        if domain is not None:
-            kwargs['domain'] = domain
-        return base_spooler(*args, **kwargs)
-
-    return spooler
-
-
-@tenantspool
-def refresh_statistics_list():
-    from combo.apps.dataviz.utils import update_available_statistics
-
-    update_available_statistics()
-
-
-@tenantspool
-def refresh_statistics_data(cell_pk, filter_params):
-    from combo.apps.dataviz.models import ChartNgCell, MissingVariable
-
-    try:
-        cell = ChartNgCell.objects.get(pk=cell_pk)
-    except ChartNgCell.DoesNotExist:
-        return
-
-    try:
-        cell.get_statistic_data(invalidate_cache=True, filter_params=filter_params)
-    except MissingVariable:
-        return
-
-    if cell.statistic.service_slug != 'bijoe':
-        cell.update_subfilters(filter_params)
+            pickle.dumps((func, args, kwargs))
+        except Exception:
+            raise ValueError(f'{(func, args, kwargs)} are unpicklable')
+        func(*args, **kwargs)

debian/control

@@ -48,7 +48,6 @@ Depends: python3-combo (= ${binary:Version}),
          python3-django-tenant-schemas,
          python3-hobo (>= 1.34),
          python3-psycopg2,
-         python3-uwsgidecorators,
          uwsgi,
          uwsgi-plugin-python3,
          ${misc:Depends},

debian/uwsgi.ini

@@ -13,8 +13,7 @@ chmod-socket = 666
 vacuum = true
 
 spooler-processes = 3
-spooler-python-import = combo.utils.spooler
-spooler-python-import = hobo.provisionning.spooler
+spooler-python-import = hobo.multitenant.spooler
 spooler-max-tasks = 20
 
 # daily

tests/test_dataviz.py

@@ -14,7 +14,6 @@ from requests.exceptions import HTTPError
 
 from combo.apps.dataviz.models import ChartFiltersCell, ChartNgCell, Gauge, Statistic, UnsupportedDataSet
 from combo.data.models import Page, ValidityInfo
-from combo.utils.spooler import refresh_statistics_data
 
 from .test_public import login
 from .utils import manager_submit_cell
@@ -3193,18 +3192,14 @@ def test_spooler_refresh_statistics_data(new_api_statistics):
     cell.filter_params = {'abc': 'def'}
     cell.save()
 
-    refresh_statistics_data(cell.pk, filter_params={'test': 'hop'})
+    cell.refresh_statistics_data(filter_params={'test': 'hop'})
     assert len(new_api_mock.call['requests']) == 1
 
     request = new_api_mock.call['requests'][0]
     assert 'abc=' not in request.url
     assert 'test=hop' in request.url
 
-    refresh_statistics_data(cell.pk, filter_params={'test': 'hop'})
-    assert len(new_api_mock.call['requests']) == 2
-
-    ChartNgCell.objects.all().delete()
-    refresh_statistics_data(cell.pk, filter_params={'test': 'hop'})
+    cell.refresh_statistics_data(filter_params={'test': 'hop'})
     assert len(new_api_mock.call['requests']) == 2
 
 
@@ -3215,7 +3210,7 @@ def test_spooler_refresh_statistics_data_bijoe(statistics):
     cell.statistic = Statistic.objects.get(slug='example')
     cell.save()
 
-    refresh_statistics_data(cell.pk, filter_params=cell.get_filter_params())
+    cell.refresh_statistics_data(filter_params=cell.get_filter_params())
     assert len(bijoe_mock.call['requests']) == 1
 
 

tests/test_spooler.py

@@ -1,52 +0,0 @@
-import importlib
-import pickle
-import sys
-from unittest import mock
-
-import pytest
-
-from combo.utils import spooler
-
-
-@pytest.fixture
-def uwsgi(monkeypatch):
-    uwsgi = mock.Mock()
-    uwsgi.opt = ['spooler']
-    uwsgi.SPOOL_OK = 1
-    monkeypatch.setitem(sys.modules, 'uwsgi', uwsgi)
-    # reload with uwsgi impl
-    importlib.reload(spooler)
-    yield uwsgi
-    monkeypatch.delitem(sys.modules, 'uwsgi')
-    importlib.reload(spooler)
-
-
-def test_tenantspool(uwsgi):
-    import uwsgidecorators
-
-    assert spooler.USE_UWSGI
-
-    mocked_f = mock.Mock()
-
-    @spooler.tenantspool
-    def function_to_spool(*args, **kwargs):
-        mocked_f(*args, **kwargs)
-
-    # check function is registered under its name, and not the name of the wrapper function
-    assert 'function_to_spool' in uwsgidecorators.spooler_functions
-
-    # check the spool request is properly serialized with tenant information
-    with mock.patch('combo.utils.spooler.connection') as connection:
-        connection.tenant.domain_url = 'example.com'
-        function_to_spool(1, b=3)
-    assert pickle.loads(uwsgi.spool.call_args[0][0][b'args']) == (1,)
-    assert pickle.loads(uwsgi.spool.call_args[0][0][b'kwargs']) == {'b': 3, 'domain': 'example.com'}
-
-    # check db is initialized, tenant restored and function called
-    with mock.patch('combo.utils.spooler.close_old_connections') as close_old_connections:
-        with mock.patch('combo.utils.spooler.tenant_context') as tenant_context:
-            uwsgidecorators.manage_spool_request(uwsgi.spool.call_args[0][0])
-            assert close_old_connections.call_count == 2
-            assert tenant_context.call_args[0][0] == 'example.com'
-            assert mocked_f.call_args[0] == (1,)
-            assert mocked_f.call_args[1] == {'b': 3}