[Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
The proposal to merge lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 -- https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
The proposal to merge lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 -- https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
Yannick Vaucher @ Camptocamp has proposed merging
lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons.
Requested reviews:
OpenERP Community Backports (ocb)
Related bugs:
Bug #1290820 in OpenERP Community Backports (Addons): report_webkit not
thread-safe: risk of document corruption
https://bugs.launchpad.net/ocb-addons/+bug/1290820
Bug #1319095 in OpenERP Community Backports (Addons): report_webkit - temp
file name can overlap
https://bugs.launchpad.net/ocb-addons/+bug/1319095
For more details, see:
https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476
--
https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476
Your team OpenERP Community Backports is requested to review the proposed merge
of lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons.
=== modified file 'report_webkit/webkit_report.py'
--- report_webkit/webkit_report.py 2014-04-16 07:43:09 +
+++ report_webkit/webkit_report.py 2014-05-14 08:51:27 +
@@ -105,9 +105,10 @@
Call webkit in order to generate pdf
if not webkit_header:
webkit_header = report_xml.webkit_header
-tmp_dir = tempfile.gettempdir()
-out_filename = tempfile.mktemp(suffix=.pdf, prefix=webkit.tmp.)
-file_to_del = [out_filename]
+out_filename = tempfile.NamedTemporaryFile(suffix=.pdf,
+ prefix=webkit.tmp.,
+ delete=False)
+file_to_del = [out_filename.name]
if comm_path:
command = [comm_path]
else:
@@ -117,25 +118,15 @@
# default to UTF-8 encoding. Use meta charset=latin-1 to override.
command.extend(['--encoding', 'utf-8'])
if header :
-head_file = file( os.path.join(
- tmp_dir,
- str(time.time()) + '.head.html'
- ),
-'w'
-)
-head_file.write(self._sanitize_html(header))
-head_file.close()
+with tempfile.NamedTemporaryFile(suffix=.head.html,
+ delete=False) as head_file:
+head_file.write(self._sanitize_html(header))
file_to_del.append(head_file.name)
command.extend(['--header-html', head_file.name])
if footer :
-foot_file = file( os.path.join(
- tmp_dir,
- str(time.time()) + '.foot.html'
- ),
-'w'
-)
-foot_file.write(self._sanitize_html(footer))
-foot_file.close()
+with tempfile.NamedTemporaryFile(suffix=.foot.html,
+ delete=False) as foot_file:
+foot_file.write(self._sanitize_html(footer))
file_to_del.append(foot_file.name)
command.extend(['--footer-html', foot_file.name])
@@ -153,13 +144,13 @@
command.extend(['--page-size', str(webkit_header.format).replace(',', '.')])
count = 0
for html in html_list :
-html_file = file(os.path.join(tmp_dir, str(time.time()) + str(count) +'.body.html'), 'w')
-count += 1
-html_file.write(self._sanitize_html(html))
-html_file.close()
+with tempfile.NamedTemporaryFile(suffix=%d.body.html %count,
+ delete=False) as html_file:
+count += 1
+html_file.write(self._sanitize_html(html))
file_to_del.append(html_file.name)
command.append(html_file.name)
-command.append(out_filename)
+command.append(out_filename.name)
stderr_fd, stderr_path = tempfile.mkstemp(text=True)
file_to_del.append(stderr_path)
try:
@@ -176,9 +167,8 @@
if status :
raise except_osv(_('Webkit error' ),
_(The command 'wkhtmltopdf' failed with error code = %s. Message: %s) % (status, error_message))
-pdf_file = open(out_filename, 'rb')
-pdf = pdf_file.read()
-pdf_file.close()
+with out_filename as pdf_file:
+pdf = pdf_file.read()
finally:
if stderr_fd is not None:
os.close(stderr_fd)
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : openerp-community-reviewer@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openerp-community-reviewer
More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
Review: Approve no test, code review Thanks for the fix. LGTM This patch will not be backportable as is to version 6.1, 6.0 as it is not Python 2.5 compliant Regards Nicolas -- https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 Your team OpenERP Community Backports is requested to review the proposed merge of lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
Review: Needs Fixing code review, no test This fix will not work on Windows. out_filename is opened on line 11 of the file, and then the pdf converter will try to open a file with the same name while the file is still open in OpenERP which will fail on windows (https://docs.python.org/2.7/library/tempfile.html?highlight=namedtemporaryfile#tempfile.NamedTemporaryFile) -- https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
Good catch -- https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons
Thanks for the review and the explanations. This has been fixed. -- https://code.launchpad.net/~camptocamp/ocb-addons/7.0-fix-1319095/+merge/219476 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
