Merge lp:~camptocamp/openobject-addons/6.0-fix1003819 into lp:openobject-addons/6.0

Proposed by Alexandre Fayolle - camptocamp
Status: Needs review
Proposed branch: lp:~camptocamp/openobject-addons/6.0-fix1003819
Merge into: lp:openobject-addons/6.0
Diff against target: 49 lines (+25/-14)
1 file modified
report_webkit/webkit_report.py (+25/-14)
To merge this branch: bzr merge lp:~camptocamp/openobject-addons/6.0-fix1003819
Reviewer Review Type Date Requested Status
Nicolas Bessi - Camptocamp (community) code review Approve
OpenERP Core Team Pending
Review via email: mp+116484@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

fix 1003819 in 6.0 branch

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

LGTM

review: Approve (code review)

Unmerged revisions

5304. By Alexandre Fayolle @ camptocamp <email address hidden>

[FIX] fix issue lp:1003819

* process the information that wkhtmltopdf can write on stderr
* cleanup the code handling file deletion to remove duplication
* provide information to the user about what went wrong

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'report_webkit/webkit_report.py'
--- report_webkit/webkit_report.py 2012-04-02 07:34:28 +0000
+++ report_webkit/webkit_report.py 2012-07-24 14:08:22 +0000
@@ -155,22 +155,33 @@
155 file_to_del.append(html_file.name)155 file_to_del.append(html_file.name)
156 command.append(html_file.name)156 command.append(html_file.name)
157 command.append(out_filename)157 command.append(out_filename)
158 stderr_fd, stderr_path = tempfile.mkstemp(text=True)
159 file_to_del.append(stderr_path)
158 try:160 try:
159 status = subprocess.call(command, stderr=subprocess.PIPE) # ignore stderr161 status = subprocess.call(command, stderr=stderr_fd)
162 os.close(stderr_fd) # ensure flush before reading
163 stderr_fd = None # avoid closing again in finally block
164 fobj = open(stderr_path, 'r')
165 error_message = fobj.read()
166 fobj.close()
167 if not error_message:
168 error_message = _('No diagnosis message was provided')
169 else:
170 error_message = _('The following diagnosis message was provided:\n') + error_message
160 if status :171 if status :
161 raise except_osv(172 raise except_osv(_('Webkit error' ),
162 _('Webkit raise an error' ), 173 _("The command 'wkhtmltopdf' failed with error code = %s. Message: %s") % (status, error_message))
163 status174 pdf_file = open(out_filename, 'rb')
164 )175 pdf = pdf_file.read()
165 except Exception:176 pdf_file.close()
166 for f_to_del in file_to_del :177 finally:
167 os.unlink(f_to_del)178 if stderr_fd is not None:
168179 os.close(stderr_fd)
169 pdf = file(out_filename, 'rb').read()180 for f_to_del in file_to_del:
170 for f_to_del in file_to_del :181 try:
171 os.unlink(f_to_del)182 os.unlink(f_to_del)
172183 except (OSError, IOError), exc:
173 os.unlink(out_filename)184 logger.error('cannot remove file %s: %s', f_to_del, exc)
174 return pdf185 return pdf
175 186
176 187