Code review comment for lp:~elbati/report-print-send/adding_base_report_to_printer

Hello,

Thanks for your proposal.

You will find my comments below, only "Blocking" points need to be fixed before merging existing modules. But fixing the other point will allow the module to be merged for V7.0

[Blockin]
- Guewen is right in as you create your own cursor it must be ensured that they are closed especially in threaded task

[Medium]
- You use the lpr command without exception management it will be also nice to use subprocess module to manage timeout
- Some public functions does not match signatures, and/or return none
- It will be nice to put in doc page or in __openerp__.py dependencies CUPS, lpr and python pycups and for pip user on libcups2-dev
- Context in named args must be None
- The update printer wizard should raise an except_osv if cups raise an exception. It is not intuitive for end user.

[Nice to have]
- Maybe link access ir_model_access_printingprinterall0 to employee group
- There are unused import and some unused variable, coding standard problem. A simple cleanup with pylint, pyflakes would be nice
- Imports should be done form opener namespace from openerp.addons.xx import yy from openerp.tools import xxx
- Absolute import are nice from . import xxx
- Model/TransientModel class should be used instead of osv.osv/osv.osv_memory