Run cli tests again - remove debug statements

.github/workflows/build.yml

@@ -134,46 +134,8 @@ jobs:
 
              # use loadscope to avoid issues running tests concurrently that share scoped fixtures
             pytest -sv tests/integration -n auto --ignore=tests/integration/synapseclient/test_command_line_client.py --dist loadscope
-          fi
-      - name: run-integration-tests-cli
-        shell: bash
-
-        # keep versions consistent with the first and last from the strategy matrix
-        if: ${{ contains(fromJSON('["99999.9"]'), matrix.python) }}
-        run: |
-          if [ -z "${{ secrets.encrypted_d17283647768_key }}" ]  || [ -z "${{ secrets.encrypted_d17283647768_key }}" ]; then
-            echo "No test configuration decryption keys available, skipping integration tests"
-
-          else
-
-            # decrypt the encrypted test synapse configuration
-            openssl aes-256-cbc -K ${{ secrets.encrypted_d17283647768_key }} -iv ${{ secrets.encrypted_d17283647768_iv }} -in test.synapseConfig.enc -out test.synapseConfig -d
-            mv test.synapseConfig ~/.synapseConfig
-
-            if [ "${{ startsWith(matrix.os, 'ubuntu') }}" == "true" ]; then
-              # on linux only we can build and run a docker container to serve as an SFTP host for our SFTP tests.
-              # Docker is not available on GH Action runners on Mac and Windows.
-
-              docker build -t sftp_tests - < tests/integration/synapseclient/core/upload/Dockerfile_sftp
-              docker run -d sftp_tests:latest
-
-              # get the internal IP address of the just launched container
-              export SFTP_HOST=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' $(docker ps -q))
-
-              printf "[sftp://$SFTP_HOST]\nusername: test\npassword: test\n" >> ~/.synapseConfig
-
-              # add to known_hosts so the ssh connections can be made without any prompting/errors
-              mkdir -p ~/.ssh
-              ssh-keyscan -H $SFTP_HOST >> ~/.ssh/known_hosts
-            fi
-
-            # set env vars used in external bucket tests from secrets
-            export EXTERNAL_S3_BUCKET_NAME="${{secrets.EXTERNAL_S3_BUCKET_NAME}}"
-            export EXTERNAL_S3_BUCKET_AWS_ACCESS_KEY_ID="${{secrets.EXTERNAL_S3_BUCKET_AWS_ACCESS_KEY_ID}}"
-            export EXTERNAL_S3_BUCKET_AWS_SECRET_ACCESS_KEY="${{secrets.EXTERNAL_S3_BUCKET_AWS_SECRET_ACCESS_KEY}}"
-
-            # use loadscope to avoid issues running tests concurrently that share scoped fixtures
-            pytest -sv tests/integration/synapseclient/test_command_line_client.py -n auto --dist loadscope
+            # Execute the CLI tests in a non-dist way because they were causing some test instability when being run concurrently
+            pytest -sv tests/integration/synapseclient/test_command_line_client.py
           fi
 
   # enforce the code matches the Black code style

synapseclient/client.py

@@ -4845,7 +4845,6 @@ def restGET(
 
         :returns: JSON encoding of response
         """
-        self.logger.warning(f"Calling restGET: {uri}")
         response = self._rest_call(
             "get", uri, None, endpoint, headers, retryPolicy, requests_session, **kwargs
         )
@@ -4874,7 +4873,6 @@ def restPOST(
 
         :returns: JSON encoding of response
         """
-        self.logger.warning(f"Calling restPOST: {uri}")
         response = self._rest_call(
             "post",
             uri,
@@ -4910,7 +4908,6 @@ def restPUT(
 
         :returns: JSON encoding of response
         """
-        self.logger.warning(f"Calling restPUT: {uri}")
         response = self._rest_call(
             "put", uri, body, endpoint, headers, retryPolicy, requests_session, **kwargs
         )
@@ -4935,7 +4932,6 @@ def restDELETE(
         :param kwargs:              Any other arguments taken by a
                                     `requests <http://docs.python-requests.org/en/latest/>`_ method
         """
-        self.logger.warning(f"Calling restDELETE: {uri}")
         self._rest_call(
             "delete",
             uri,

synapseclient/core/upload/multipart_upload.py

@@ -658,10 +658,7 @@ def _multipart_upload(
             # success
             return upload_status_response["resultFileHandleId"]
 
-        except SynapseUploadFailedException as ex:
-            syn.logger.error(
-                f"Failed in multipart upload. [retry: {retry < MAX_RETRIES}, Error: {str(ex)}]"
-            )
+        except SynapseUploadFailedException:
             if retry < MAX_RETRIES:
                 retry += 1
             else:

tests/unit/synapseutils/unit_test_synapseutils_sync.py

@@ -26,7 +26,7 @@
 from synapseclient.core.pool_provider import get_executor
 
 
-def test_readManifest__sync_order_with_home_directory(syn):
+def test_readManifest__sync_order_with_home_directory(syn: Synapse):
     """SYNPY-508"""
 
     # row1's file depends on row2's file but is listed first
@@ -63,7 +63,7 @@ def test_readManifest__sync_order_with_home_directory(syn):
         )
 
 
-def test_readManifestFile__synapseStore_values_not_set(syn):
+def test_readManifestFile__synapseStore_values_not_set(syn: Synapse):
     project_id = "syn123"
     header = "path\tparent\n"
     path1 = os.path.abspath(os.path.expanduser("~/file1.txt"))
@@ -89,7 +89,7 @@ def test_readManifestFile__synapseStore_values_not_set(syn):
         assert expected_synapseStore == actual_synapseStore
 
 
-def test_readManifestFile__synapseStore_values_are_set(syn):
+def test_readManifestFile__synapseStore_values_are_set(syn: Synapse):
     project_id = "syn123"
     header = "path\tparent\tsynapseStore\n"
     path1 = os.path.abspath(os.path.expanduser("~/file1.txt"))
@@ -129,23 +129,23 @@ def test_readManifestFile__synapseStore_values_are_set(syn):
         assert expected_synapseStore == actual_synapseStore
 
 
-def test_syncFromSynapse__non_file_entity(syn):
+def test_syncFromSynapse__non_file_entity(syn: Synapse):
     table_schema = "syn12345"
     with patch.object(syn, "getChildren", return_value=[]), patch.object(
         syn, "get", return_value=Schema(name="asssdfa", parent="whatever")
     ):
         pytest.raises(ValueError, synapseutils.syncFromSynapse, syn, table_schema)
 
 
-def test_syncFromSynapse__empty_folder(syn):
+def test_syncFromSynapse__empty_folder(syn: Synapse):
     folder = Folder(name="the folder", parent="whatever", id="syn123")
     with patch.object(syn, "getChildren", return_value=[]), patch.object(
         syn, "get", return_value=Folder(name="asssdfa", parent="whatever")
     ):
         assert list() == synapseutils.syncFromSynapse(syn, folder)
 
 
-def test_syncFromSynapse__file_entity(syn):
+def test_syncFromSynapse__file_entity(syn: Synapse):
     file = File(name="a file", parent="some parent", id="syn456")
     with patch.object(
         syn, "getChildren", return_value=[file]
@@ -154,7 +154,7 @@ def test_syncFromSynapse__file_entity(syn):
         patch_syn_get_children.assert_not_called()
 
 
-def test_syncFromSynapse__folder_contains_one_file(syn):
+def test_syncFromSynapse__folder_contains_one_file(syn: Synapse):
     folder = Folder(name="the folder", parent="whatever", id="syn123")
     file = File(name="a file", parent=folder, id="syn456")
     with patch.object(
@@ -164,7 +164,7 @@ def test_syncFromSynapse__folder_contains_one_file(syn):
         patch_syn_get_children.called_with(folder["id"])
 
 
-def test_syncFromSynapse__project_contains_empty_folder(syn):
+def test_syncFromSynapse__project_contains_empty_folder(syn: Synapse):
     project = Project(name="the project", parent="whatever", id="syn123")
     file = File(name="a file", parent=project, id="syn456")
     folder = Folder(name="a folder", parent=project, id="syn789")
@@ -194,7 +194,7 @@ def syn_get_side_effect(entity, *args, **kwargs):
         )
 
 
-def test_syncFromSynapse__downloadFile_is_false(syn):
+def test_syncFromSynapse__downloadFile_is_false(syn: Synapse):
     """
     Verify when passing the argument downloadFile is equal to False,
     syncFromSynapse won't download the file to clients' local end.
@@ -228,7 +228,7 @@ def syn_get_side_effect(entity, *args, **kwargs):
 @patch.object(synapseutils.sync, "generateManifest")
 @patch.object(synapseutils.sync, "_get_file_entity_provenance_dict")
 def test_syncFromSynapse__manifest_is_all(
-    mock__get_file_entity_provenance_dict, mock_generateManifest, syn
+    mock__get_file_entity_provenance_dict, mock_generateManifest, syn: Synapse
 ):
     """
     Verify manifest argument equal to "all" that pass in to syncFromSynapse, it will create root_manifest and all
@@ -297,7 +297,7 @@ def syn_get_side_effect(entity, *args, **kwargs):
 @patch.object(synapseutils.sync, "generateManifest")
 @patch.object(synapseutils.sync, "_get_file_entity_provenance_dict")
 def test_syncFromSynapse__manifest_is_root(
-    mock__get_file_entity_provenance_dict, mock_generateManifest, syn
+    mock__get_file_entity_provenance_dict, mock_generateManifest, syn: Synapse
 ):
     """
     Verify manifest argument equal to "root" that pass in to syncFromSynapse, it will create root_manifest file only.
@@ -359,7 +359,7 @@ def syn_get_side_effect(entity, *args, **kwargs):
 @patch.object(synapseutils.sync, "generateManifest")
 @patch.object(synapseutils.sync, "_get_file_entity_provenance_dict")
 def test_syncFromSynapse__manifest_is_suppress(
-    mock__get_file_entity_provenance_dict, mock_generateManifest, syn
+    mock__get_file_entity_provenance_dict, mock_generateManifest, syn: Synapse
 ):
     """
     Verify manifest argument equal to "suppress" that pass in to syncFromSynapse, it won't create any manifest file.