From 4cd8886d95f2d23dc78406589aa1e83b3156ed2f Mon Sep 17 00:00:00 2001 From: BryanFauble <17128019+BryanFauble@users.noreply.github.com> Date: Mon, 30 Oct 2023 13:10:27 -0700 Subject: [PATCH] code review feedback --- synapseclient/core/upload/multipart_upload.py | 13 +------------ synapseclient/core/utils.py | 12 ++++++++++++ .../integration/synapseclient/core/test_caching.py | 2 +- .../synapseclient/test_command_line_client.py | 2 +- .../core/upload/unit_test_multipart_upload.py | 7 +------ 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/synapseclient/core/upload/multipart_upload.py b/synapseclient/core/upload/multipart_upload.py index bd63650e5..68ba0eb03 100644 --- a/synapseclient/core/upload/multipart_upload.py +++ b/synapseclient/core/upload/multipart_upload.py @@ -10,7 +10,6 @@ import concurrent.futures from contextlib import contextmanager -import hashlib import json import math import mimetypes @@ -30,7 +29,7 @@ SynapseUploadAbortedException, SynapseUploadFailedException, ) -from synapseclient.core.utils import md5_for_file, MB, Spinner +from synapseclient.core.utils import md5_fn, md5_for_file, MB, Spinner # AWS limits MAX_NUMBER_OF_PARTS = 10000 @@ -478,11 +477,6 @@ def multipart_upload_file( def part_fn(part_number): return _get_file_chunk(file_path, part_number, part_size) - def md5_fn(part, _): - md5 = hashlib.new("md5", usedforsecurity=False) - md5.update(part) - return md5.hexdigest() - return _multipart_upload( syn, dest_file_name, @@ -530,11 +524,6 @@ def multipart_upload_string( https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.17 """ - def md5_fn(part, _): - md5 = hashlib.new("md5", usedforsecurity=False) - md5.update(part) - return md5.hexdigest() - data = text.encode("utf-8") file_size = len(data) md5_hex = md5_fn(data, None) diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py index 6b0fbd7b5..699581b87 100644 --- a/synapseclient/core/utils.py +++ b/synapseclient/core/utils.py @@ -57,6 +57,18 @@ def md5_for_file(filename, block_size=2 * MB, callback=None): return md5 +def md5_fn(part, _): + """Calculate the MD5 of a file-like object. + + :part -- A file-like object to read from. + + :returns: The MD5 + """ + md5 = hashlib.new("md5", usedforsecurity=False) + md5.update(part) + return md5.hexdigest() + + def download_file(url, localFilepath=None): """ Downloads a remote file. diff --git a/tests/integration/synapseclient/core/test_caching.py b/tests/integration/synapseclient/core/test_caching.py index 54e30bde4..440c335a6 100644 --- a/tests/integration/synapseclient/core/test_caching.py +++ b/tests/integration/synapseclient/core/test_caching.py @@ -181,7 +181,7 @@ def thread_get_and_update_file_from_Project( syn.logger.warning( f"thread_get_and_update_file_from_Project()::get_all_ids_from_Project timed out, [project: {project.id}]" ) - if len(id) <= 0: + if len(id) == 0: sleep_for_a_bit() continue diff --git a/tests/integration/synapseclient/test_command_line_client.py b/tests/integration/synapseclient/test_command_line_client.py index 2b5053fa9..7ec2426cb 100644 --- a/tests/integration/synapseclient/test_command_line_client.py +++ b/tests/integration/synapseclient/test_command_line_client.py @@ -615,7 +615,7 @@ def test_command_get_recursive_and_query(test_state): schema1 = test_state.syn.store( Schema( - name="Foo Table_test_command_get_recursive_and_query", + name=str(uuid.uuid4()), columns=cols, parent=project_entity, ) diff --git a/tests/unit/synapseclient/core/upload/unit_test_multipart_upload.py b/tests/unit/synapseclient/core/upload/unit_test_multipart_upload.py index 31368d2bd..9ea7ee4af 100644 --- a/tests/unit/synapseclient/core/upload/unit_test_multipart_upload.py +++ b/tests/unit/synapseclient/core/upload/unit_test_multipart_upload.py @@ -23,12 +23,7 @@ pool_provider, UploadAttempt, ) - - -def md5_fn(part, _): - md5 = hashlib.new("md5", usedforsecurity=False) - md5.update(part) - return md5.hexdigest() +from synapseclient.core.utils import md5_fn class TestUploadAttempt: