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
1=== modified file 'report_webkit/webkit_report.py'
2--- report_webkit/webkit_report.py 2012-04-02 07:34:28 +0000
3+++ report_webkit/webkit_report.py 2012-07-24 14:08:22 +0000
4@@ -155,22 +155,33 @@
5 file_to_del.append(html_file.name)
6 command.append(html_file.name)
7 command.append(out_filename)
8+ stderr_fd, stderr_path = tempfile.mkstemp(text=True)
9+ file_to_del.append(stderr_path)
10 try:
11- status = subprocess.call(command, stderr=subprocess.PIPE) # ignore stderr
12+ status = subprocess.call(command, stderr=stderr_fd)
13+ os.close(stderr_fd) # ensure flush before reading
14+ stderr_fd = None # avoid closing again in finally block
15+ fobj = open(stderr_path, 'r')
16+ error_message = fobj.read()
17+ fobj.close()
18+ if not error_message:
19+ error_message = _('No diagnosis message was provided')
20+ else:
21+ error_message = _('The following diagnosis message was provided:\n') + error_message
22 if status :
23- raise except_osv(
24- _('Webkit raise an error' ),
25- status
26- )
27- except Exception:
28- for f_to_del in file_to_del :
29- os.unlink(f_to_del)
30-
31- pdf = file(out_filename, 'rb').read()
32- for f_to_del in file_to_del :
33- os.unlink(f_to_del)
34-
35- os.unlink(out_filename)
36+ raise except_osv(_('Webkit error' ),
37+ _("The command 'wkhtmltopdf' failed with error code = %s. Message: %s") % (status, error_message))
38+ pdf_file = open(out_filename, 'rb')
39+ pdf = pdf_file.read()
40+ pdf_file.close()
41+ finally:
42+ if stderr_fd is not None:
43+ os.close(stderr_fd)
44+ for f_to_del in file_to_del:
45+ try:
46+ os.unlink(f_to_del)
47+ except (OSError, IOError), exc:
48+ logger.error('cannot remove file %s: %s', f_to_del, exc)
49 return pdf
50
51