Repository: Duke-GCB/DukeDSClient

Issue:
Check for Deprecations
Deprecated python 2 modules and features will eventually be removed from python 3. Once we drop python2 support we should check our code explained here: https://docs.python.org/3.9/whatsnew/3.9.html#you-should-check-for-deprecationwarning-in-your-code

Issue #304 removes python2 support.

Discussion:

Patch:
diff --git a/ddsc/core/ddsapi.py b/ddsc/core/ddsapi.py
index 295b0c8..8db2fe7 100644
--- a/ddsc/core/ddsapi.py
+++ b/ddsc/core/ddsapi.py
@@ -278,6 +278,10 @@ class DataServiceApi(object):
         """
         self.http = requests.Session()
 
+    def close(self):
+        self.http.close()
+        self.http = None
+
     def _url_parts(self, url_suffix, data, content_type):
         """
         Format the url data based on config_type.
diff --git a/ddsc/core/fileuploader.py b/ddsc/core/fileuploader.py
index ba342fe..0924589 100644
--- a/ddsc/core/fileuploader.py
+++ b/ddsc/core/fileuploader.py
@@ -363,6 +363,7 @@ def upload_async(data_service_auth_data, config, upload_id,
     except:
         error_msg = "".join(traceback.format_exception(*sys.exc_info()))
         progress_queue.error(error_msg)
+    data_service.close()
 
 
 class ChunkSender(object):
diff --git a/ddsc/core/projectuploader.py b/ddsc/core/projectuploader.py
index 15c2b53..39cd3cc 100644
--- a/ddsc/core/projectuploader.py
+++ b/ddsc/core/projectuploader.py
@@ -312,6 +312,7 @@ def upload_project_run(upload_context):
     data_service = upload_context.make_data_service()
     project_name = upload_context.project_name_or_id.get_name_or_raise()
     result = data_service.create_project(project_name, project_name)
+    data_service.close()
     return result.json()['id']
 
 
@@ -363,6 +364,7 @@ def upload_folder_run(upload_context):
     data_service = upload_context.make_data_service()
     folder_name, parent_kind, parent_remote_id = upload_context.params
     result = data_service.create_folder(folder_name, parent_kind, parent_remote_id)
+    data_service.close()
     return result.json()['id']
 
 
@@ -489,7 +491,9 @@ def create_small_file(upload_context):
     upload_id, url_info = upload_operations.create_upload_and_chunk_url(
         upload_context.project_id, path_data, hash_data, storage_provider_id=upload_context.config.storage_provider_id)
     upload_operations.send_file_external(url_info, chunk)
-    return upload_operations.finish_upload(upload_id, hash_data, parent_data, remote_file_id)
+    file_response_json = upload_operations.finish_upload(upload_id, hash_data, parent_data, remote_file_id)
+    data_service.close()
+    return file_response_json
 
 
 class ProjectUploadDryRun(object):
diff --git a/ddsc/core/upload.py b/ddsc/core/upload.py
index 524d61e..f2fbd34 100644
--- a/ddsc/core/upload.py
+++ b/ddsc/core/upload.py
@@ -67,6 +67,9 @@ class ProjectUpload(object):
         url = '{}: {}'.format(msg, self.remote_store.data_service.portal_url(project_id))
         return url
 
+    def cleanup(self):
+        self.remote_store.close()
+
 
 class UploadReport(object):
     """
diff --git a/ddsc/ddsclient.py b/ddsc/ddsclient.py
index cc3f9d3..dfb1b2d 100644
--- a/ddsc/ddsclient.py
+++ b/ddsc/ddsclient.py
@@ -17,6 +17,7 @@ from ddsc.config import create_config
 from ddsc.sdk.client import Client
 from ddsc.core.download import ProjectFileDownloader
 
+
 NO_PROJECTS_FOUND_MESSAGE = 'No projects found.'
 INVALID_DELIVERY_RECIPIENT_MSG = 'Delivery recipient cannot be a share user. Remove recipient from --share-users and try again.'
 TWO_SECONDS = 2
@@ -88,6 +89,7 @@ class DDSClient(object):
         self.show_error_stack_trace = config.debug_mode
         command = command_constructor(config)
         command.run(args)
+        command.cleanup()
 
 
 class BaseCommand(object):
@@ -139,6 +141,9 @@ class BaseCommand(object):
                 to_users.append(user)
         return to_users
 
+    def cleanup(self):
+        self.remote_store.close()
+
 
 class ClientCommand(object):
     def __init__(self, config):
@@ -150,6 +155,9 @@ class ClientCommand(object):
         else:
             return self.client.get_project_by_id(args.project_id)
 
+    def cleanup(self):
+        self.client.close()
+
 
 class UploadCommand(BaseCommand):
     """
@@ -204,6 +212,7 @@ class UploadCommand(BaseCommand):
                 print(upload_report.get_content())
                 print('\n')
             print(project_upload.get_url_msg())
+            project_upload.cleanup()
 
 
 class DownloadCommand(ClientCommand):
diff --git a/ddsc/sdk/client.py b/ddsc/sdk/client.py
index bcbbaa2..70834f4 100644
--- a/ddsc/sdk/client.py
+++ b/ddsc/sdk/client.py
@@ -77,6 +77,9 @@ class Client(object):
         """
         return self.dds_connection.get_file_by_id(file_id)
 
+    def close(self):
+        self.dds_connection.close()
+
 
 class DDSConnection(object):
     """
@@ -89,6 +92,9 @@ class DDSConnection(object):
         self.config = config
         self.data_service = DataServiceApi(DataServiceAuth(config), config.url)
 
+    def close(self):
+        self.data_service.close()
+
     def _create_array_response(self, resp, array_item_constructor):
         items = resp.json()['results']
         return [array_item_constructor(self, data_dict) for data_dict in items]
diff --git a/setup.py b/setup.py
index 7b00f45..1c3141c 100644
--- a/setup.py
+++ b/setup.py
@@ -15,7 +15,7 @@ setup(name='DukeDSClient',
           'pytz',
           'future',
           'six',
-          'tenacity==5.0.4',
+          'tenacity==6.2.0',
         ],
         test_suite='nose.collector',
         tests_require=['nose', 'mock'],


Test patch:
diff --git a/ddsc/core/remotestore.py b/ddsc/core/remotestore.py
index 70911ac..b2aca87 100644
--- a/ddsc/core/remotestore.py
+++ b/ddsc/core/remotestore.py
@@ -320,6 +320,9 @@ class RemoteStore(object):
         """
         return RemoteFileUrl(self.data_service.get_file_url(file_id).json())
 
+    def close(self):
+        self.data_service.close()
+
 
 class RemoteProject(object):
     """
diff --git a/ddsc/core/tests/test_ddsapi.py b/ddsc/core/tests/test_ddsapi.py
index 3c580d0..0551043 100644
--- a/ddsc/core/tests/test_ddsapi.py
+++ b/ddsc/core/tests/test_ddsapi.py
@@ -488,6 +488,14 @@ class TestDataServiceApi(TestCase):
         self.assertIsNotNone(api.http)
         self.assertEqual(type(api.http), requests.sessions.Session)
 
+    @patch('ddsc.core.ddsapi.requests')
+    def test_close(self, mock_requests):
+        api = DataServiceApi(auth=self.create_mock_auth(config_page_size=100), url="something.com/v1/", http=None)
+        self.assertIsNotNone(api.http)
+        api.close()
+        self.assertIsNone(api.http)
+        mock_requests.Session.return_value.close.assert_called_with()
+
     def test_check_err_with_good_response(self):
         resp = Mock(headers={}, status_code=202)
         url_suffix = ""
diff --git a/ddsc/core/tests/test_fileuploader.py b/ddsc/core/tests/test_fileuploader.py
index 02f6c25..c7046f0 100644
--- a/ddsc/core/tests/test_fileuploader.py
+++ b/ddsc/core/tests/test_fileuploader.py
@@ -51,7 +51,8 @@ class TestParallelChunkProcessor(TestCase):
 
 class TestUploadAsync(TestCase):
     @patch('ddsc.core.fileuploader.ChunkSender')
-    def test_upload_async_sends_exception_to_progress_queue(self, mock_chunk_sender):
+    @patch('ddsc.core.fileuploader.DataServiceApi')
+    def test_upload_async_sends_exception_to_progress_queue(self, mock_data_service_api, mock_chunk_sender):
         data_service_auth_data = MagicMock()
         config = MagicMock()
         upload_id = 123
@@ -65,6 +66,7 @@ class TestUploadAsync(TestCase):
         params = progress_queue.error.call_args
         positional_args = params[0]
         self.assertIn('Something Failed!', positional_args[0])
+        mock_data_service_api.return_value.close.assert_called_with()
 
 
 class TestFileUploadOperations(TestCase):
diff --git a/ddsc/core/tests/test_projectuploader.py b/ddsc/core/tests/test_projectuploader.py
index 3c090f8..f0173cf 100644
--- a/ddsc/core/tests/test_projectuploader.py
+++ b/ddsc/core/tests/test_projectuploader.py
@@ -2,7 +2,7 @@ from unittest import TestCase
 import pickle
 import multiprocessing
 from ddsc.core.projectuploader import UploadSettings, UploadContext, ProjectUploadDryRun, CreateProjectCommand, \
-    upload_project_run, create_small_file, ProjectUploader, HashFileCommand, CreateSmallFileCommand
+    upload_project_run, create_small_file, ProjectUploader, HashFileCommand, CreateSmallFileCommand, upload_folder_run
 from ddsc.core.util import KindType
 from ddsc.core.remotestore import ProjectNameOrId
 from mock import MagicMock, Mock, patch, call, ANY
@@ -169,6 +169,19 @@ class TestCreateProjectCommand(TestCase):
         mock_upload_context.project_name_or_id = ProjectNameOrId.create_from_name('mouse')
         upload_project_run(mock_upload_context)
         mock_data_service.create_project.assert_called_with('mouse', 'mouse')
+        mock_data_service.close.assert_called_with()
+
+
+class TestCreateFolderCommand(TestCase):
+    def test_upload_folder_run(self):
+        context = Mock(params=["data", "dds-project", "1234"])
+        mock_data_service = context.make_data_service.return_value
+        mock_data_service.create_folder.return_value.json.return_value = {"id": "5678"}
+        upload_id = upload_folder_run(context)
+
+        self.assertEqual(upload_id, "5678")
+        mock_data_service.create_folder.assert_called_with("data", "dds-project", "1234")
+        mock_data_service.close.assert_called_with()
 
 
 class TestCreateSmallFile(TestCase):
@@ -187,6 +200,7 @@ class TestCreateSmallFile(TestCase):
 
         self.assertEqual(resp, mock_file_operations.return_value.finish_upload.return_value)
         mock_file_operations.return_value.create_file_chunk_url.assert_not_called()
+        upload_context.make_data_service.return_value.close.assert_called_with()
 
     @patch('ddsc.core.projectuploader.FileUploadOperations', autospec=True)
     def test_create_small_file_hash_matches(self, mock_file_operations):
diff --git a/ddsc/core/tests/test_remotestore.py b/ddsc/core/tests/test_remotestore.py
index 878435b..07fda03 100644
--- a/ddsc/core/tests/test_remotestore.py
+++ b/ddsc/core/tests/test_remotestore.py
@@ -345,6 +345,12 @@ class TestRemoteAuthRole(TestCase):
 
 
 class TestRemoteStore(TestCase):
+    @patch('ddsc.core.remotestore.DataServiceApi')
+    def test_close(self, mock_data_service_api):
+        remote_store = RemoteStore(config=MagicMock())
+        remote_store.close()
+        mock_data_service_api.return_value.close.assert_called_with()
+
     def test_auth_roles_system(self):
         JSON_DATA = {
             "results": [
diff --git a/ddsc/sdk/tests/test_client.py b/ddsc/sdk/tests/test_client.py
index 6d0f475..219a2a8 100644
--- a/ddsc/sdk/tests/test_client.py
+++ b/ddsc/sdk/tests/test_client.py
@@ -125,6 +125,13 @@ class TestClient(TestCase):
         self.assertEqual(folder, some_file)
         mock_dss_connection.return_value.get_file_by_id.assert_called_with('456')
 
+    @patch('ddsc.sdk.client.create_config')
+    @patch('ddsc.sdk.client.DDSConnection')
+    def test_close(self, mock_dss_connection, mock_create_config):
+        client = Client()
+        client.close()
+        mock_dss_connection.return_value.close.assert_called_with()
+
 
 class TestDDSConnection(TestCase):
     @patch('ddsc.sdk.client.DataServiceApi')
diff --git a/ddsc/tests/test_ddsclient.py b/ddsc/tests/test_ddsclient.py
index 584caf7..f72f784 100644
--- a/ddsc/tests/test_ddsclient.py
+++ b/ddsc/tests/test_ddsclient.py
@@ -2,7 +2,7 @@ from __future__ import absolute_import
 from unittest import TestCase
 from ddsc.ddsclient import BaseCommand, UploadCommand, ListCommand, DownloadCommand, ClientCommand, MoveCommand
 from ddsc.ddsclient import ShareCommand, DeliverCommand, InfoCommand, read_argument_file_contents, \
-    INVALID_DELIVERY_RECIPIENT_MSG
+    INVALID_DELIVERY_RECIPIENT_MSG, DDSClient
 from mock import patch, MagicMock, Mock, call, ANY
 
 
@@ -70,6 +70,13 @@ class TestBaseCommand(TestCase):
         self.assertEqual('Invalid',
                          str(raisedError.exception))
 
+    @patch('ddsc.ddsclient.RemoteStore', autospec=True)
+    def test_cleanup(self, mock_remote_store):
+        mock_config = MagicMock()
+        base_cmd = BaseCommand(mock_config)
+        base_cmd.cleanup()
+        mock_remote_store.return_value.close.assert_called_with()
+
 
 class TestUploadCommand(TestCase):
     @patch("ddsc.ddsclient.ProjectUpload")
@@ -113,6 +120,7 @@ class TestUploadCommand(TestCase):
             call('\n'),
             call(mock_project_upload.return_value.get_url_msg.return_value),
         ])
+        mock_project_upload.return_value.cleanup.assert_called_with()
 
     @patch("ddsc.ddsclient.ProjectUpload")
     @patch("ddsc.ddsclient.ProjectNameOrId")
@@ -360,6 +368,15 @@ class TestDDSClient(TestCase):
         with open("setup.py") as infile:
             self.assertIn("setup(", read_argument_file_contents(infile))
 
+    @patch('ddsc.ddsclient.create_config')
+    def test_run_command(self, mock_create_config):
+        ddsclient = DDSClient()
+        command_constructor = Mock()
+        mock_args = Mock(allow_insecure_config_file=False)
+        ddsclient._run_command(command_constructor, mock_args)
+        command_constructor.return_value.run.assert_called_with(mock_args)
+        command_constructor.return_value.cleanup.assert_called_with()
+
 
 class TestListCommand(TestCase):
     @patch('sys.stdout.write')
@@ -462,6 +479,12 @@ class TestClientCommand(TestCase):
         self.assertEqual(project, mock_client.return_value.get_project_by_id.return_value)
         mock_client.return_value.get_project_by_id.assert_called_with('123abc')
 
+    @patch('ddsc.ddsclient.Client')
+    def test_cleanup(self, mock_client):
+        client_command = ClientCommand(config=Mock())
+        client_command.cleanup()
+        mock_client.return_value.close.assert_called_with()
+
 
 class TestMoveCommand(TestCase):
     @patch('ddsc.ddsclient.Client')