Merge lp:~camptocamp/carriers-deliveries/7.0-threaded-dispatch-label-generation into lp:~stock-logistic-core-editors/carriers-deliveries/7.0

[Yannick Vaucher @ Camptocamp]

delivery_carrier_label_dispatch - Improve efficiency of label generation by threading it. For instance for postlogistics, using serialized calls to the SOAP web service is a really slow as it would wait for a response before sending the next request.

Also improve exception feed back. And thus exception are non blocking for all label generation. You will only have to relaunch generation for failed ones.

[lp:~camptocamp/carriers-deliveries/7.0-threaded-dispatch-label-generation]

42. By Yannick Vaucher @ Camptocamp on 2014-04-10

removed conflict garbage

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello,

Thank for the patch

Small PEP8 in manifest also noticed that
the title underline is not correct

[Link module] Carrier labels - Picking dispatch
==============================

It should go to the end of previous line

sys is imported but not used

remark
q_except can be a simpler data cohntainer like a list has they are thread safe.

the nested try catch structure here is strange

        try:
            try:
                picking_out_obj.generate_labels(
                    thread_cr, uid, [picking.id],
                    tracking_ids=tracking_ids,
                    context=context)
            except orm.except_orm as e:
                # add information on picking and pack in the exception
                picking_name = _('Picking: %s') % picking.name
                pack_num = _('Pack: %s') % pack.name if pack else ''
                raise orm.except_orm(
                    e.name,
                    _('%s %s - %s') % (picking_name, pack_num, e.value)) ...

review: Needs Fixing

[lp:~camptocamp/carriers-deliveries/7.0-threaded-dispatch-label-generation]

43. By Yannick Vaucher @ Camptocamp on 2014-05-14

Fix rst title in manifest

44. By Yannick Vaucher @ Camptocamp on 2014-05-14

remove unused import

45. By Yannick Vaucher @ Camptocamp on 2014-05-14

[PEP8]

46. By Yannick Vaucher @ Camptocamp on 2014-05-14

rewrite exception handling for cursor and orm exceptions on generate_labels

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Thanks for the review.

I made the fixes.

I have also rewritten the exception handling, thought it does exactly the same thing. I have to use nested exception to ensure the rollback is played in anycase. Then I want to complete informations on the orm exceptions.

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

s/thought/though

[lp:~camptocamp/carriers-deliveries/7.0-threaded-dispatch-label-generation]

47. By Yannick Vaucher @ Camptocamp on 2014-06-13

Specify Exception class to ensure catching right exceptions and avoid catching SystemExit and KeyboardInterupt exceptions

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

fixed except: -> except Exception:

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

LGTM

review: Approve

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Note to myself

This project is now hosted on https://github.com/OCA/carriers-deliveries. Please move your proposal there if you still want to merge it once fixed. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

Unmerged revisions

47. By Yannick Vaucher @ Camptocamp on 2014-06-13

Specify Exception class to ensure catching right exceptions and avoid catching SystemExit and KeyboardInterupt exceptions

46. By Yannick Vaucher @ Camptocamp on 2014-05-14

rewrite exception handling for cursor and orm exceptions on generate_labels

45. By Yannick Vaucher @ Camptocamp on 2014-05-14

[PEP8]

44. By Yannick Vaucher @ Camptocamp on 2014-05-14

remove unused import

43. By Yannick Vaucher @ Camptocamp on 2014-05-14

Fix rst title in manifest

42. By Yannick Vaucher @ Camptocamp on 2014-04-10

removed conflict garbage

41. By Yannick Vaucher @ Camptocamp on 2014-04-10

delivery_carrier_label_dispatch - Improve efficiency of label generation by threading it. For instance for postlogistics, using serialized calls to the SOAP web service is a really slow as it would wait for a response before sending the next request.

40. By Yannick Vaucher @ Camptocamp on 2014-04-10

merge lp:~camptocamp/carriers-deliveries/7.0-delivery_carrier_label_dispatch-output-file-yvr

=== modified file 'delivery_carrier_label_dispatch/__openerp__.py'
--- delivery_carrier_label_dispatch/__openerp__.py	2014-06-13 14:52:12 +0000
+++ delivery_carrier_label_dispatch/__openerp__.py	2014-06-13 14:52:12 +0000
@@ -27,7 +27,7 @@
  'depends': ['base_delivery_carrier_label', 'picking_dispatch'],
  'description': """
 [Link module] Carrier labels - Picking dispatch
-==============================
+===============================================
 
 This module adds a wizard on picking dispatch to generate the labels
 of the packs. The labels are merged in one PDF file.
@@ -37,6 +37,19 @@
 
 If you don't define your pack it will be considered a picking is a single pack.
 
+
+Tips
+----
+For picking dispatch with huge number of labels to generate your can add a
+number of worker with ir.config_parameter `shipping_label.num_workers` to
+parallelize the generation on multiple workers.
+This can be really useful for exemple using PostLogistics web service to
+gain speed.
+
+Be careful not to set too many worker as each one will need to instanciate
+a cursor on database and this could generate PoolErrors.
+A good choice would be to set it as `db_maxconn` / number_of_worker / 2
+
 Contributors
 ------------
 
 
=== modified file 'delivery_carrier_label_dispatch/wizard/generate_labels.py'
--- delivery_carrier_label_dispatch/wizard/generate_labels.py	2014-06-13 14:52:12 +0000
+++ delivery_carrier_label_dispatch/wizard/generate_labels.py	2014-06-13 14:52:12 +0000
@@ -20,13 +20,20 @@
 ##############################################################################
 from operator import attrgetter
 from itertools import groupby
+import Queue
+import threading
+import logging
 
+from openerp import pooler
 from openerp.osv import orm, fields
 from openerp.tools.translate import _
 
 from ..pdf_utils import assemble_pdf
 
 
+_logger = logging.getLogger(__name__)
+
+
 class DeliveryCarrierLabelGenerate(orm.TransientModel):
 
     _name = 'delivery.carrier.label.generate'
@@ -85,37 +92,138 @@
             return None
         return label_obj.browse(cr, uid, label_id[0], context=context)
 
+    def _do_generate_labels(self, cr, uid, wizard, pack, picking,
+                            label, context=None):
+        """ Generate a label in a thread safe context
+
+        Here we declare a specific cursor so do not launch
+        too many threads
+        """
+        picking_out_obj = self.pool['stock.picking.out']
+        # generate the label of the pack
+        tracking_ids = [pack.id] if pack else None
+        # create a cursor to be thread safe
+        thread_cr = pooler.get_db(cr.dbname).cursor()
+        try:
+            picking_out_obj.generate_labels(
+                thread_cr, uid, [picking.id],
+                tracking_ids=tracking_ids,
+                context=context)
+            thread_cr.commit()
+        except Exception:
+            thread_cr.rollback()
+            try:
+                raise
+            except orm.except_orm as e:
+                # add information on picking and pack in the exception
+                picking_name = _('Picking: %s') % picking.name
+                pack_num = _('Pack: %s') % pack.name if pack else ''
+                raise orm.except_orm(
+                    e.name,
+                    _('%s %s - %s') % (picking_name, pack_num, e.value))
+        finally:
+            thread_cr.close()
+
+    def worker(self, q, q_except):
+        """ A worker to generate labels
+
+        Takes data from queue q
+
+        And if the worker encounters errors, he will add them in
+        q_except queue
+        """
+        while not q.empty():
+            args, kwargs = q.get()
+            try:
+                self._do_generate_labels(*args, **kwargs)
+            except Exception as e:
+                q_except.put(e)
+            finally:
+                q.task_done()
+
+    def _get_num_workers(self, cr, uid, context=None):
+        """ Get number of worker parameter for labels generation
+
+        Optional ir.config_parameter is `shipping_label.num_workers`
+        """
+        param_obj = self.pool['ir.config_parameter']
+        num_workers = param_obj.get_param(cr, uid,
+                                          'shipping_label.num_workers')
+        if not num_workers:
+            return 1
+        return int(num_workers)
+
     def _get_all_pdf(self, cr, uid, wizard, dispatch, context=None):
+        q = Queue.Queue()
+        q_except = Queue.Queue()
+
+        # create the tasks to generate labels
         for pack, moves, label in self._get_packs(cr, uid, wizard, dispatch,
                                                   context=context):
             if not label or wizard.generate_new_labels:
-                picking_out_obj = self.pool['stock.picking.out']
                 picking = moves[0].picking_id
-                # generate the label of the pack
-                if pack:
-                    tracking_ids = [pack.id]
-                else:
-                    tracking_ids = None
-                try:
-                    picking_out_obj.generate_labels(
-                        cr, uid, [picking.id],
-                        tracking_ids=tracking_ids,
-                        context=context)
-                except orm.except_orm as e:
-                    picking_name = _('Picking: %s') % picking.name
-                    pack_num = _('Pack: %s') % pack.name if pack else ''
-                    raise orm.except_orm(
-                        e.name,
-                        _('%s %s - %s') % (picking_name, pack_num, e.value))
-                if pack:
-                    label = self._find_pack_label(cr, uid, wizard, pack,
-                                                  context=context)
-                else:
-                    label = self._find_picking_label(cr, uid, wizard, picking,
-                                                     context=context)
-                if not label:
-                    continue  # no label could be generated
+                args = (cr, uid, wizard, pack, picking, label)
+                kwargs = {'context': context}
+                task = (args, kwargs)
+                q.put(task)
+
+        # create few workers to parallelize label generation
+        num_workers = self._get_num_workers(cr, uid, context=context)
+        _logger.info('Starting %s workers to generate labels', num_workers)
+        for i in range(num_workers):
+            t = threading.Thread(target=self.worker, args=(q, q_except))
+            t.daemon = True
+            t.start()
+
+        # wait for all tasks to be done
+        q.join()
+
+        # We will not create a partial PDF if some labels weren't
+        # generated thus we raise catched exceptions by the workers
+        # We will try to regroup all orm exception in one
+        if not q_except.empty():
+
+            error_count = {}
+            messages = []
+            while not q_except.empty():
+                e = q_except.get()
+                if isinstance(e, orm.except_orm):
+                    if not e.name in error_count:
+                        error_count[e.name] = 1
+                    else:
+                        error_count[e.name] += 1
+                    messages.append(e.value)
+                else:
+                    # raise other exceptions like PoolError if
+                    # too many cursor where created by workers
+                    raise e
+            titles = []
+            for key, v in error_count.iteritems():
+                titles.append('%sx %s' % (v, key))
+
+            title = _('Errors while generating labels: ') + ' '.join(titles)
+            message = _('Some labels couldn\'t be generated. Please correct '
+                        'following errors and generate labels again to create '
+                        'the ones which failed.\n\n'
+                        ) + '\n'.join(messages)
+            raise orm.except_orm(title, message)
+
+        # create a new cursor to be up to date with what was created by workers
+        join_cr = pooler.get_db(cr.dbname).cursor()
+        for pack, moves, label in self._get_packs(join_cr, uid,
+                                                  wizard, dispatch,
+                                                  context=context):
+            picking = moves[0].picking_id
+            if pack:
+                label = self._find_pack_label(join_cr, uid, wizard, pack,
+                                              context=context)
+            else:
+                label = self._find_picking_label(join_cr, uid, wizard, picking,
+                                                 context=context)
+            if not label:
+                continue  # no label could be generated
             yield label
+        join_cr.close()
 
     def action_generate_labels(self, cr, uid, ids, context=None):
         """
@@ -128,7 +236,7 @@
         if not this.dispatch_ids:
             raise orm.except_orm(_('Error'), _('No picking dispatch selected'))
 
-        attachment_obj = self.pool.get('ir.attachment')
+        attachment_obj = self.pool['ir.attachment']
 
         for dispatch in this.dispatch_ids:
             labels = self._get_all_pdf(cr, uid, this, dispatch,