OpenERP Community Backports (Addons)

Merge lp://staging/~camptocamp/ocb-addons/7.0-fix-1319095 into lp://staging/ocb-addons

7.0-fix-1319095
Merge into 7.0

Proposed by Yannick Vaucher @ Camptocamp on 2014-05-14

Status:

Merged

Approved by:

Leonardo Pistone on 2014-05-21

Approved revision:

no longer in the source branch.

Merged at revision:

10208

Proposed branch:

lp://staging/~camptocamp/ocb-addons/7.0-fix-1319095

Merge into:

lp://staging/ocb-addons

Diff against target:

74 lines (+15/-25)

1 file modified

report_webkit/webkit_report.py (+15/-25)

To merge this branch:

bzr merge lp://staging/~camptocamp/ocb-addons/7.0-fix-1319095

Related bugs:

Bug #1290820: report_webkit not thread-safe: risk of document corruption	Undecided	Fix Released
Bug #1319095: report_webkit - temp file name can overlap	Undecided	New

Link a bug report

Reviewer	Review Type	Date Requested	Status
Pedro Manuel Baeza	code review		Approve on 2014-05-14
Alexandre Fayolle - camptocamp	code review, no test	2014-05-14	Approve on 2014-05-14
Nicolas Bessi - Camptocamp (community)	no test, code review	2014-05-14	Approve on 2014-05-14
Florent		2014-05-14	Pending
Review via email: mp+219476@code.staging.launchpad.net

Description of the change

Use NamedTemporaryFile instead of file and of deprecated mktemp. That way we ensure 2 files created at the exact same time will have a unique name

Another hot fix from Florent was made here https://code.launchpad.net/~florent.x/openobject-addons/trunk-1290820-report_webkit/+merge/210387

I believe this one is a bit cleaner as it refactor a bit the code. However it doesn't keep timestamp in file name. But do we need timestamp in file name ?

If so I can still add it in prefix or suffix.

Revision history for this message

Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote on 2014-05-14:

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

review: Approve (no test, code review)

Revision history for this message

Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote on 2014-05-14:

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)