Merge lp:~camptocamp/account-financial-report/7.0-sync-afr_webkit-with-report_webkit into lp:~account-report-core-editor/account-financial-report/7.0

Proposed by Yannick Vaucher @ Camptocamp
Status: Merged
Merged at revision: 92
Proposed branch: lp:~camptocamp/account-financial-report/7.0-sync-afr_webkit-with-report_webkit
Merge into: lp:~account-report-core-editor/account-financial-report/7.0
Diff against target: 226 lines (+52/-44)
1 file modified
account_financial_report_webkit/report/webkit_parser_header_fix.py (+52/-44)
To merge this branch: bzr merge lp:~camptocamp/account-financial-report/7.0-sync-afr_webkit-with-report_webkit
Reviewer Review Type Date Requested Status
Stéphane Bidoul (Acsone) (community) Approve
Guewen Baconnier @ Camptocamp tested Approve
Pedro Manuel Baeza code review Approve
Nicolas Bessi - Camptocamp (community) Approve
Review via email: mp+223058@code.launchpad.net

Description of the change

Adapt account_financial_report_webkit to fixes in addons

https://github.com/odoo/odoo/pull/55
https://github.com/odoo/odoo/pull/56

Plus few improvement following pep8 to remove useless variable and import

To post a comment you must log in.
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

LGTM, taking in account that previous comment on source MP are still valid.

review: Approve
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

LGTM.

Regards.

review: Approve (code review)
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Thanks

review: Approve (tested)
Revision history for this message
Stéphane Bidoul (Acsone) (sbi) wrote :

Hi,

Is it just me or this MP broke afr with ocb 7.0?

Traceback (most recent call last):
File "/home/openerp/instance/account-financial-report/account_financial_report_webkit/report/webkit_parser_header_fix.py", line 228, in create_single_pdf
**parser_instance.localcontext)
File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/template.py", line 443, in render
return runtime._render(self, self.callable_, args, data)
File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/runtime.py", line 803, in _render
**_kwargs_for_callable(callable_, data))
File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/runtime.py", line 835, in _render_context
_exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/runtime.py", line 860, in _exec_template
callable_(context, *args, **kwargs)
File "memory:0x5184dd0", line 22, in render_body
<%
TypeError: translate_call() takes exactly 2 arguments (3 given)

review: Needs Fixing (test)
Revision history for this message
Stéphane Bidoul (Acsone) (sbi) wrote :

reverting to rev 91 works

Revision history for this message
Stéphane Bidoul (Acsone) (sbi) wrote :

I'm using rev 5343 of ocb-server/7.0, which the latest on launchpad

Not sure what's going on. Unfortunately I can't dig deeper right now.

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

On 06/17/2014 09:29 PM, Stéphane Bidoul (Acsone) wrote:
> Review: Needs Fixing test
>
> Hi,
>
> Is it just me or this MP broke afr with ocb 7.0?
>
> Traceback (most recent call last):
> File "/home/openerp/instance/account-financial-report/account_financial_report_webkit/report/webkit_parser_header_fix.py", line 228, in create_single_pdf
> **parser_instance.localcontext)
> File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/template.py", line 443, in render
> return runtime._render(self, self.callable_, args, data)
> File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/runtime.py", line 803, in _render
> **_kwargs_for_callable(callable_, data))
> File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/runtime.py", line 835, in _render_context
> _exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
> File "/home/openerp/instance/eggs/Mako-1.0.0-py2.7.egg/mako/runtime.py", line 860, in _exec_template
> callable_(context, *args, **kwargs)
> File "memory:0x5184dd0", line 22, in render_body
> <%
> TypeError: translate_call() takes exactly 2 arguments (3 given)
>
>

It is necessary to be compatible with this change:
https://github.com/odoo/odoo/pull/56
That has been merged in official branches.

I didn't noticed that it was not working in ocb because in my local OCB
branch (which is a bit out of date), the patch was there:

revno: 10194 [merge]
author: <email address hidden>
committer: Leonardo Pistone <email address hidden>
branch nick: 7.0-ocb
timestamp: Wed 2014-05-21 10:58:28 +0200
message:
  Transmit parser_instance using partial function for mako translation
callback.

  This to remove parser_instance from attribute as it can cause race
condition
  on cursors.

But it is no longer there when I look in the current ocb-addons tree.

I guess we should propose the MP again on OCB

--
Guewen Baconnier
Business Solutions Software Developer

Camptocamp SA
PSE A, CH-1015 Lausanne
Phone: +41 21 619 10 39
Office: +41 21 619 10 10
http://www.camptocamp.com/

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

This has been fixed by Pedro, some revision were overwritten last week and are now back in OCB.

Thanks for noticing it Stéphane

Revision history for this message
Stéphane Bidoul (Acsone) (sbi) wrote :

Tested again with ocb-addons/7.0 HEAD.

Situation is recovered. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_financial_report_webkit/report/webkit_parser_header_fix.py'
2--- account_financial_report_webkit/report/webkit_parser_header_fix.py 2014-03-18 14:55:32 +0000
3+++ account_financial_report_webkit/report/webkit_parser_header_fix.py 2014-06-13 12:07:01 +0000
4@@ -30,8 +30,8 @@
5 import os
6 import subprocess
7 import tempfile
8-import time
9 import logging
10+from functools import partial
11
12
13 from mako import exceptions
14@@ -48,11 +48,12 @@
15 # Class used only as a workaround to bug:
16 # http://code.google.com/p/wkhtmltopdf/issues/detail?id=656
17
18-# html headers and footers do not work on big files (hundreds of pages) so we replace them by
19-# text headers and footers passed as arguments to wkhtmltopdf
20+# html headers and footers do not work on big files (hundreds of pages) so we
21+# replace them by text headers and footers passed as arguments to wkhtmltopdf
22 # this class has to be removed once the bug is fixed
23
24-# in your report class, to print headers and footers as text, you have to add them in the localcontext with a key 'additional_args'
25+# in your report class, to print headers and footers as text, you have to add
26+# them in the localcontext with a key 'additional_args'
27 # for instance:
28 # header_report_name = _('PARTNER LEDGER')
29 # footer_date_time = self.formatLang(str(datetime.today()), date_time=True)
30@@ -75,23 +76,25 @@
31 from mako.template import Template
32 from mako.lookup import TemplateLookup
33
34+
35 def mako_template(text):
36 """Build a Mako template.
37
38 This template uses UTF-8 encoding
39 """
40- tmp_lookup = TemplateLookup() #we need it in order to allow inclusion and inheritance
41+ tmp_lookup = TemplateLookup() # we need it in order to allow inclusion and inheritance
42 return Template(text, input_encoding='utf-8', output_encoding='utf-8', lookup=tmp_lookup)
43
44
45 class HeaderFooterTextWebKitParser(webkit_report.WebKitParser):
46
47- def generate_pdf(self, comm_path, report_xml, header, footer, html_list, webkit_header=False):
48+ def generate_pdf(self, comm_path, report_xml, header, footer, html_list,
49+ webkit_header=False, parser_instance=False):
50 """Call webkit in order to generate pdf"""
51 if not webkit_header:
52 webkit_header = report_xml.webkit_header
53- tmp_dir = tempfile.gettempdir()
54- out_filename = tempfile.mktemp(suffix=".pdf", prefix="webkit.tmp.")
55+ fd, out_filename = tempfile.mkstemp(suffix=".pdf",
56+ prefix="webkit.tmp.")
57 file_to_del = [out_filename]
58 if comm_path:
59 command = [comm_path]
60@@ -115,16 +118,16 @@
61 if webkit_header.format:
62 command.extend(['--page-size', str(webkit_header.format).replace(',', '.')])
63
64- if self.parser_instance.localcontext.get('additional_args', False):
65- for arg in self.parser_instance.localcontext['additional_args']:
66+ if parser_instance.localcontext.get('additional_args', False):
67+ for arg in parser_instance.localcontext['additional_args']:
68 command.extend(arg)
69
70 count = 0
71 for html in html_list:
72- html_file = file(os.path.join(tmp_dir, str(time.time()) + str(count) +'.body.html'), 'w')
73- count += 1
74- html_file.write(html)
75- html_file.close()
76+ with tempfile.NamedTemporaryFile(suffix="%d.body.html" % count,
77+ delete=False) as html_file:
78+ count += 1
79+ html_file.write(self._sanitize_html(html))
80 file_to_del.append(html_file.name)
81 command.append(html_file.name)
82 command.append(out_filename)
83@@ -132,8 +135,8 @@
84 file_to_del.append(stderr_path)
85 try:
86 status = subprocess.call(command, stderr=stderr_fd)
87- os.close(stderr_fd) # ensure flush before reading
88- stderr_fd = None # avoid closing again in finally block
89+ os.close(stderr_fd) # ensure flush before reading
90+ stderr_fd = None # avoid closing again in finally block
91 fobj = open(stderr_path, 'r')
92 error_message = fobj.read()
93 fobj.close()
94@@ -142,11 +145,11 @@
95 else:
96 error_message = _('The following diagnosis message was provided:\n') + error_message
97 if status:
98- raise except_osv(_('Webkit error' ),
99+ raise except_osv(_('Webkit error'),
100 _("The command 'wkhtmltopdf' failed with error code = %s. Message: %s") % (status, error_message))
101- pdf_file = open(out_filename, 'rb')
102- pdf = pdf_file.read()
103- pdf_file.close()
104+ with open(out_filename, 'rb') as pdf_file:
105+ pdf = pdf_file.read()
106+ os.close(fd)
107 finally:
108 if stderr_fd is not None:
109 os.close(stderr_fd)
110@@ -160,57 +163,60 @@
111 # override needed to keep the attachments' storing procedure
112 def create_single_pdf(self, cursor, uid, ids, data, report_xml, context=None):
113 """generate the PDF"""
114+
115 if context is None:
116- context={}
117+ context = {}
118 htmls = []
119 if report_xml.report_type != 'webkit':
120- return super(HeaderFooterTextWebKitParser,self).create_single_pdf(cursor, uid, ids, data, report_xml, context=context)
121+ return super(HeaderFooterTextWebKitParser, self
122+ ).create_single_pdf(cursor, uid, ids, data,
123+ report_xml, context=context)
124
125- self.parser_instance = self.parser(cursor,
126- uid,
127- self.name2,
128- context=context)
129+ parser_instance = self.parser(cursor,
130+ uid,
131+ self.name2,
132+ context=context)
133
134 self.pool = pooler.get_pool(cursor.dbname)
135 objs = self.getObjects(cursor, uid, ids, context)
136- self.parser_instance.set_context(objs, data, ids, report_xml.report_type)
137+ parser_instance.set_context(objs, data, ids, report_xml.report_type)
138
139- template = False
140+ template = False
141
142 if report_xml.report_file:
143 path = addons.get_module_resource(*report_xml.report_file.split(os.path.sep))
144 if os.path.exists(path):
145 template = file(path).read()
146 if not template and report_xml.report_webkit_data:
147- template = report_xml.report_webkit_data
148+ template = report_xml.report_webkit_data
149 if not template:
150 raise except_osv(_('Error!'), _('Webkit Report template not found !'))
151 header = report_xml.webkit_header.html
152- footer = report_xml.webkit_header.footer_html
153+
154 if not header and report_xml.header:
155 raise except_osv(
156 _('No header defined for this Webkit report!'),
157- _('Please set a header in company settings')
158+ _('Please set a header in company settings.')
159 )
160
161 css = report_xml.webkit_header.css
162 if not css:
163 css = ''
164- user = self.pool.get('res.users').browse(cursor, uid, uid)
165
166+ translate_call = partial(self.translate_call, parser_instance)
167 #default_filters=['unicode', 'entity'] can be used to set global filter
168 body_mako_tpl = mako_template(template)
169 helper = WebKitHelper(cursor, uid, report_xml.id, context)
170 if report_xml.precise_mode:
171 for obj in objs:
172- self.parser_instance.localcontext['objects'] = [obj]
173+ parser_instance.localcontext['objects'] = [obj]
174 try:
175 html = body_mako_tpl.render(helper=helper,
176 css=css,
177- _=self.translate_call,
178- **self.parser_instance.localcontext)
179+ _=translate_call,
180+ **parser_instance.localcontext)
181 htmls.append(html)
182- except Exception, e:
183+ except Exception:
184 msg = exceptions.text_error_template().render()
185 _logger.error(msg)
186 raise except_osv(_('Webkit render'), msg)
187@@ -218,15 +224,16 @@
188 try:
189 html = body_mako_tpl.render(helper=helper,
190 css=css,
191- _=self.translate_call,
192- **self.parser_instance.localcontext)
193+ _=translate_call,
194+ **parser_instance.localcontext)
195 htmls.append(html)
196- except Exception, e:
197+ except Exception:
198 msg = exceptions.text_error_template().render()
199 _logger.error(msg)
200 raise except_osv(_('Webkit render'), msg)
201
202- # NO html footer and header because we write them as text with wkhtmltopdf
203+ # NO html footer and header because we write them as text with
204+ # wkhtmltopdf
205 head = foot = False
206
207 if report_xml.webkit_debug:
208@@ -234,13 +241,14 @@
209 deb = body_mako_tpl.render(helper=helper,
210 css=css,
211 _debug=tools.ustr("\n".join(htmls)),
212- _=self.translate_call,
213- **self.parser_instance.localcontext)
214- except Exception, e:
215+ _=translate_call,
216+ **parser_instance.localcontext)
217+ except Exception:
218 msg = exceptions.text_error_template().render()
219 _logger.error(msg)
220 raise except_osv(_('Webkit render'), msg)
221 return (deb, 'html')
222 bin = self.get_lib(cursor, uid)
223- pdf = self.generate_pdf(bin, report_xml, head, foot, htmls)
224+ pdf = self.generate_pdf(bin, report_xml, head, foot, htmls,
225+ parser_instance=parser_instance)
226 return (pdf, 'pdf')

Subscribers

People subscribed via source and target branches

to status/vote changes: