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
- 7.0-sync-afr_webkit-with-report_webkit
- Merge into 7.0
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 |
Related bugs: |
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 |
Commit message
Description of the change
Adapt account_
https:/
https:/
Plus few improvement following pep8 to remove useless variable and import
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote : | # |
Pedro Manuel Baeza (pedro.baeza) wrote : | # |
LGTM.
Regards.
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote : | # |
Thanks
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/
**parser_
File "/home/
return runtime.
File "/home/
**_kwargs_
File "/home/
_exec_template(
File "/home/
callable_(context, *args, **kwargs)
File "memory:0x5184dd0", line 22, in render_body
<%
TypeError: translate_call() takes exactly 2 arguments (3 given)
Stéphane Bidoul (Acsone) (sbi) wrote : | # |
reverting to rev 91 works
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.
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/
> **parser_
> File "/home/
> return runtime.
> File "/home/
> **_kwargs_
> File "/home/
> _exec_template(
> File "/home/
> 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:/
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://
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
Stéphane Bidoul (Acsone) (sbi) wrote : | # |
Tested again with ocb-addons/7.0 HEAD.
Situation is recovered. Thanks!
Preview Diff
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') |
LGTM, taking in account that previous comment on source MP are still valid.