Simplify export_file_compute_checksum_md5 function #411
Conversation
| usefile = Path(tmpdir.name) / "tmpfile" | ||
|
|
||
| assert usefile is not None | ||
| export_file(obj, usefile, extension, flag=flag) |
There was a problem hiding this comment.
If export_file(...) raises an Exception, and we are using a tmp file it wont get cleaned up. The new code will do so by usage of the context manger.
| @@ -906,17 +906,14 @@ def export( | |||
|
|
|||
| obj = self._check_obj_if_file(obj) | |||
| logger.info("Export to file and compute MD5 sum, using flag: <%s>", useflag) | |||
| toutfile, md5 = export_file_compute_checksum_md5( | |||
| # inject md5 checksum in metadata | |||
| metadata["file"]["checksum_md5"] = export_file_compute_checksum_md5( | |||
| obj, | |||
| outfile, | |||
| outfile.suffix, | |||
| flag=useflag, # type: ignore | |||
There was a problem hiding this comment.
Needs more investigation.
Atm. i think yes. But i think we should be able to get this hash from a in memory stream insted. |
|
A more thorough go-through of the whole checksum is perhaps needed. We are discussing need for enabling consumers (i.e. REP in this case) to use it to verify correct data in their end, which means they would have to be able to reproduce it regardless of storage (so checksum of data cannot depend on the specific file format is is stored in). Also I cannot find a single test for this (which means we never tested this?). I assume current changes will have no impact on output, but would be nice to confirm with tests (and in the process increase coverage). |
Addresses #410