Merge lp:~camptocamp/openobject-addons/trunk-fix1003819 into lp:openobject-addons/6.1

Proposed by Alexandre Fayolle - camptocamp on 2012-07-24
Status: Needs review
Proposed branch: lp:~camptocamp/openobject-addons/trunk-fix1003819
Merge into: lp:openobject-addons/6.1
Diff against target: 60 lines (+26/-15)
1 file modified
report_webkit/webkit_report.py (+26/-15)
To merge this branch: bzr merge lp:~camptocamp/openobject-addons/trunk-fix1003819
Reviewer Review Type Date Requested Status
Nicolas Bessi - Camptocamp (community) code review only 2012-07-24 Approve on 2014-05-01
Guewen Baconnier @ Camptocamp (community) 2012-07-24 Approve on 2012-07-24
OpenERP Core Team 2012-07-24 Pending
Review via email: mp+116470@code.launchpad.net

Description of the change

To post a comment you must log in.

We must merge your changeset :-)

review: Approve
6915. By Alexandre Fayolle @ camptocamp <email address hidden> on 2012-07-24

[FIX] report_webkit: ensure the temp file fd is always closed

I added related merge requests on the trunk/7.0 branch and on the 6.0 branch

LGTM

review: Approve (code review only)

Unmerged revisions

6915. By Alexandre Fayolle @ camptocamp <email address hidden> on 2012-07-24

[FIX] report_webkit: ensure the temp file fd is always closed

6914. By Alexandre Fayolle @ camptocamp <email address hidden> on 2012-07-24

[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-02-16 10:58:58 +0000
+++ report_webkit/webkit_report.py 2012-07-24 13:36:21 +0000
@@ -107,7 +107,7 @@
107 tmp_dir = tempfile.gettempdir()107 tmp_dir = tempfile.gettempdir()
108 out_filename = tempfile.mktemp(suffix=".pdf", prefix="webkit.tmp.")108 out_filename = tempfile.mktemp(suffix=".pdf", prefix="webkit.tmp.")
109 files = []109 files = []
110 file_to_del = []110 file_to_del = [out_filename]
111 if comm_path:111 if comm_path:
112 command = [comm_path]112 command = [comm_path]
113 else:113 else:
@@ -160,22 +160,33 @@
160 file_to_del.append(html_file.name)160 file_to_del.append(html_file.name)
161 command.append(html_file.name)161 command.append(html_file.name)
162 command.append(out_filename)162 command.append(out_filename)
163 stderr_fd, stderr_path = tempfile.mkstemp(text=True)
164 file_to_del.append(stderr_path)
163 try:165 try:
164 status = subprocess.call(command, stderr=subprocess.PIPE) # ignore stderr166 status = subprocess.call(command, stderr=stderr_fd)
167 os.close(stderr_fd) # ensure flush before reading
168 stderr_fd = None # avoid closing again in finally block
169 fobj = open(stderr_path, 'r')
170 error_message = fobj.read()
171 fobj.close()
172 if not error_message:
173 error_message = _('No diagnosis message was provided')
174 else:
175 error_message = _('The following diagnosis message was provided:\n') + error_message
165 if status :176 if status :
166 raise except_osv(177 raise except_osv(_('Webkit error' ),
167 _('Webkit raise an error' ),178 _("The command 'wkhtmltopdf' failed with error code = %s. Message: %s") % (status, error_message))
168 status179 pdf_file = open(out_filename, 'rb')
169 )180 pdf = pdf_file.read()
170 except Exception:181 pdf_file.close()
171 for f_to_del in file_to_del :182 finally:
172 os.unlink(f_to_del)183 if stderr_fd is not None:
173184 os.close(stderr_fd)
174 pdf = file(out_filename, 'rb').read()185 for f_to_del in file_to_del:
175 for f_to_del in file_to_del :186 try:
176 os.unlink(f_to_del)187 os.unlink(f_to_del)
177188 except (OSError, IOError), exc:
178 os.unlink(out_filename)189 logger.error('cannot remove file %s: %s', f_to_del, exc)
179 return pdf190 return pdf
180191
181 def translate_call(self, src):192 def translate_call(self, src):