Refactors amclient.py

* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py
* Initial changes provides scope for future refactoring as required
* Addresses #47

.gitignore

@@ -10,5 +10,14 @@
 # Sublime files
 *.sublime-*
 
+# Log files
 *.log.[0-9]
+*.log
+
+# Tox files
 .tox/
+
+# Script generated files
+*.db
+*.lck
+

requirements/base.txt

@@ -2,3 +2,4 @@ metsrw==0.2.0
 requests<3.0
 sqlalchemy
 six
+urllib3

tests/test_amclient.py

@@ -1,4 +1,3 @@
-#!/usr/bin/env python
 """To run the tests::
 
     $ python -m unittest tests.test_amclient
@@ -11,6 +10,7 @@
 import vcr
 
 from transfers import amclient
+from transfers import errors
 
 
 AM_URL = 'http://192.168.168.192'
@@ -24,6 +24,7 @@
 
 
 class TmpDir:
+
     """Context manager to clear and create a temporary directory and destroy it
     after usage.
     """
@@ -110,21 +111,22 @@ def test_close_completed_transfers_no_transfers(self):
         assert isinstance(close_succeeded, list)
         assert len(close_succeeded) == 0
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/completed_transfers_bad_key.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'completed_transfers_bad_key.yaml')
     def test_completed_transfers_bad_key(self):
         """Test getting completed transfers when a bad AM API key is
         provided.
         """
         completed_transfers = amclient.AMClient(
             am_api_key='bad api key', am_user_name=AM_USER_NAME,
             am_url=AM_URL).completed_transfers()
-        assert completed_transfers is None
+        assert completed_transfers is errors.ERR_INVALID_RESPONSE
 
     @vcr.use_cassette(
         'fixtures/vcr_cassettes/unapproved_transfers_transfers.yaml')
     def test_unapproved_transfers_transfers(self):
-        """Test getting unapproved transfers when there are unapproved transfers
-        to get.
+        """Test getting unapproved transfers when there are
+        unapproved transfers to get.
         """
         unapproved_transfers = amclient.AMClient(
             am_api_key=AM_API_KEY, am_user_name=AM_USER_NAME,
@@ -320,8 +322,8 @@ def test_download_dip_dip(self):
                 ss_api_key=SS_API_KEY,
                 directory=TMP_DIR).download_dip()
             assert (dip_path ==
-                    '{}/package-c0e37bab-e51e-482d-a066-a277330de9a7.7z'.format(
-                        TMP_DIR))
+                    '{}/package-c0e37bab-e51e-482d-a066-a277330de9a7.7z'
+                    .format(TMP_DIR))
             assert os.path.isfile(dip_path)
 
     @vcr.use_cassette('fixtures/vcr_cassettes/download_dip_no_dip.yaml')

tests/test_transfers.py

@@ -6,6 +6,7 @@
 from sqlalchemy.orm import sessionmaker
 import vcr
 
+from transfers import errors
 from transfers import transfer
 from transfers import models
 
@@ -33,135 +34,189 @@ class TestAutomateTransfers(unittest.TestCase):
     def test_get_status_transfer(self):
         transfer_uuid = 'dfc8cf5f-b5b1-408c-88b1-34215964e9d6'
         transfer_name = 'test1'
-        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid, 'transfer', session)
+        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid,
+                                   'transfer', session)
         assert isinstance(info, dict)
         assert info['status'] == 'USER_INPUT'
         assert info['type'] == 'transfer'
         assert info['name'] == transfer_name
         assert info['uuid'] == transfer_uuid
         assert info['directory'] == transfer_name
-        assert info['path'] == '/var/archivematica/sharedDirectory/watchedDirectories/activeTransfers/standardTransfer/test1/'
+        assert info['path'] == ('/var/archivematica/sharedDirectory/'
+                                'watchedDirectories/activeTransfers/'
+                                'standardTransfer/test1/')
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_status_transfer_to_ingest.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'get_status_transfer_to_ingest.yaml')
     def test_get_status_transfer_to_ingest(self):
         # Reference values
         transfer_uuid = 'dfc8cf5f-b5b1-408c-88b1-34215964e9d6'
         unit_name = 'test1'
         sip_uuid = 'f2248e2a-b593-43db-b60c-fa8513021785'
         # Setup transfer in DB
-        new_transfer = models.Unit(uuid=transfer_uuid, path=b'/foo', unit_type='transfer', status='PROCESSING', current=True)
+        new_transfer = models.Unit(uuid=transfer_uuid,
+                                   path=b'/foo',
+                                   unit_type='transfer',
+                                   status='PROCESSING',
+                                   current=True)
         session.add(new_transfer)
         session.commit()
 
         # Run test
-        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid, 'transfer', session)
+        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid,
+                                   'transfer', session)
         # Verify
         assert isinstance(info, dict)
         assert info['status'] == 'USER_INPUT'
         assert info['type'] == 'SIP'
         assert info['name'] == unit_name
         assert info['uuid'] == sip_uuid
         assert info['directory'] == unit_name + '-' + sip_uuid
-        assert info['path'] == '/var/archivematica/sharedDirectory/watchedDirectories/workFlowDecisions/selectFormatIDToolIngest/test1-f2248e2a-b593-43db-b60c-fa8513021785/'
+        assert info['path'] == ('/var/archivematica/sharedDirectory/'
+                                'watchedDirectories/workFlowDecisions/'
+                                'selectFormatIDToolIngest/'
+                                'test1-f2248e2a-b593-43db-b60c-fa8513021785/')
 
     @vcr.use_cassette('fixtures/vcr_cassettes/get_status_ingest.yaml')
     def test_get_status_ingest(self):
         sip_uuid = 'f2248e2a-b593-43db-b60c-fa8513021785'
         sip_name = 'test1'
-        info = transfer.get_status(AM_URL, USER, API_KEY, sip_uuid, 'ingest', session)
+        info = transfer.get_status(AM_URL, USER, API_KEY, sip_uuid,
+                                   'ingest', session)
         assert isinstance(info, dict)
         assert info['status'] == 'USER_INPUT'
         assert info['type'] == 'SIP'
         assert info['name'] == sip_name
         assert info['uuid'] == sip_uuid
         assert info['directory'] == sip_name + '-' + sip_uuid
-        assert info['path'] == '/var/archivematica/sharedDirectory/watchedDirectories/workFlowDecisions/selectFormatIDToolIngest/test1-f2248e2a-b593-43db-b60c-fa8513021785/'
+        assert info['path'] == ('/var/archivematica/sharedDirectory/'
+                                'watchedDirectories/workFlowDecisions/'
+                                'selectFormatIDToolIngest/'
+                                'test1-f2248e2a-b593-43db-b60c-fa8513021785/')
 
     @vcr.use_cassette('fixtures/vcr_cassettes/get_status_no_unit.yaml')
     def test_get_status_no_unit(self):
         transfer_uuid = 'deadc0de-c0de-c0de-c0de-deadc0dec0de'
-        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid, 'transfer', session)
-        assert info is None
+        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid,
+                                   'transfer', session)
+        self.assertEqual(info,
+                         errors.error_lookup(errors.ERR_INVALID_RESPONSE))
 
     @vcr.use_cassette('fixtures/vcr_cassettes/get_status_not_json.yaml')
     def test_get_status_not_json(self):
         transfer_uuid = 'dfc8cf5f-b5b1-408c-88b1-34215964e9d6'
-        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid, 'transfer', session)
-        assert info is None
+        info = transfer.get_status(AM_URL, USER, API_KEY, transfer_uuid,
+                                   'transfer', session)
+        self.assertEqual(info,
+                         errors.error_lookup(errors.ERR_INVALID_RESPONSE))
 
     def test_get_accession_id_no_script(self):
         accession_id = transfer.get_accession_id(os.path.curdir)
-        assert accession_id is None
+        self.assertEqual(accession_id, None)
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_first_run.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'get_next_transfer_first_run.yaml')
     def test_get_next_transfer_first_run(self):
         # All default values
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, TS_LOCATION_UUID, PATH_PREFIX, DEPTH, COMPLETED, FILES)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          TS_LOCATION_UUID, PATH_PREFIX,
+                                          DEPTH, COMPLETED, FILES)
         # Verify
-        assert path == b'SampleTransfers/BagTransfer'
+        self.assertEqual(path, b'SampleTransfers/BagTransfer')
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_existing_set.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'get_next_transfer_existing_set.yaml')
     def test_get_next_transfer_existing_set(self):
         # Set completed set
         completed = {b'SampleTransfers/BagTransfer'}
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, TS_LOCATION_UUID, PATH_PREFIX, DEPTH, completed, FILES)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          TS_LOCATION_UUID, PATH_PREFIX,
+                                          DEPTH, completed, FILES)
         # Verify
-        assert path == b'SampleTransfers/CSVmetadata'
+        self.assertEqual(path, b'SampleTransfers/CSVmetadata')
 
     @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_depth.yaml')
     def test_get_next_transfer_depth(self):
         # Set depth
         depth = 2
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, TS_LOCATION_UUID, PATH_PREFIX, depth, COMPLETED, FILES)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          TS_LOCATION_UUID, PATH_PREFIX,
+                                          depth, COMPLETED, FILES)
         # Verify
-        assert path == b'SampleTransfers/BagTransfer/data'
+        self.assertEqual(path, b'SampleTransfers/BagTransfer/data')
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_no_prefix.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'get_next_transfer_no_prefix.yaml')
     def test_get_next_transfer_no_prefix(self):
         # Set no prefix
         path_prefix = b''
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, TS_LOCATION_UUID, path_prefix, DEPTH, COMPLETED, FILES)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          TS_LOCATION_UUID, path_prefix,
+                                          DEPTH, COMPLETED, FILES)
         # Verify
-        assert path == b'OPF format-corpus'
+        self.assertEqual(path, b'OPF format-corpus')
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_all_complete.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'get_next_transfer_all_complete.yaml')
     def test_get_next_transfer_all_complete(self):
         # Set completed set to be all elements
-        completed = {b'SampleTransfers/BagTransfer', b'SampleTransfers/CSVmetadata', b'SampleTransfers/DigitizationOutput', b'SampleTransfers/DSpaceExport', b'SampleTransfers/Images', b'SampleTransfers/ISODiskImage', b'SampleTransfers/Multimedia', b'SampleTransfers/OCRImage', b'SampleTransfers/OfficeDocs', b'SampleTransfers/RawCameraImages', b'SampleTransfers/structMapSample'}
+        completed = {b'SampleTransfers/BagTransfer',
+                     b'SampleTransfers/CSVmetadata',
+                     b'SampleTransfers/DigitizationOutput',
+                     b'SampleTransfers/DSpaceExport',
+                     b'SampleTransfers/Images',
+                     b'SampleTransfers/ISODiskImage',
+                     b'SampleTransfers/Multimedia',
+                     b'SampleTransfers/OCRImage',
+                     b'SampleTransfers/OfficeDocs',
+                     b'SampleTransfers/RawCameraImages',
+                     b'SampleTransfers/structMapSample'}
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, TS_LOCATION_UUID, PATH_PREFIX, DEPTH, completed, FILES)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          TS_LOCATION_UUID, PATH_PREFIX,
+                                          DEPTH, completed, FILES)
         # Verify
-        assert path is None
+        self.assertEqual(path, None)
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_bad_source.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'get_next_transfer_bad_source.yaml')
     def test_get_next_transfer_bad_source(self):
         # Set bad TS Location UUID
         ts_location_uuid = 'badd8d39-9cee-495e-b7ee-5e6292549bad'
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, ts_location_uuid, PATH_PREFIX, DEPTH, COMPLETED, FILES)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          ts_location_uuid, PATH_PREFIX,
+                                          DEPTH, COMPLETED, FILES)
         # Verify
-        assert path is None
+        self.assertEqual(path,
+                         errors.error_lookup(errors.ERR_INVALID_RESPONSE))
 
     @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_files.yaml')
     def test_get_next_transfer_files(self):
         # See files
         files = True
         completed = {b'SampleTransfers/BagTransfer'}
         # Test
-        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY, TS_LOCATION_UUID, PATH_PREFIX, DEPTH, completed, files)
+        path = transfer.get_next_transfer(SS_URL, SS_USER, SS_KEY,
+                                          TS_LOCATION_UUID, PATH_PREFIX,
+                                          DEPTH, completed, files)
         # Verify
-        assert path == b'SampleTransfers/BagTransfer.zip'
+        self.assertEqual(path, b'SampleTransfers/BagTransfer.zip')
 
-    @vcr.use_cassette('fixtures/vcr_cassettes/get_next_transfer_failed_auth.yaml')
+    @vcr.use_cassette('fixtures/vcr_cassettes/'
+                      'Sget_next_transfer_failed_auth.yaml')
     def test_get_next_transfer_failed_auth(self):
         # All default values
         ss_user = 'demo'
         ss_key = 'dne'
         # Test
-        path = transfer.get_next_transfer(SS_URL, ss_user, ss_key, TS_LOCATION_UUID, PATH_PREFIX, DEPTH, COMPLETED, FILES)
+        path = transfer.get_next_transfer(SS_URL, ss_user, ss_key,
+                                          TS_LOCATION_UUID, PATH_PREFIX,
+                                          DEPTH, COMPLETED, FILES)
         # Verify
-        assert path is None
+        self.assertEqual(path,
+                         errors.error_lookup(errors.ERR_SERVER_CONN))

tox.ini

@@ -19,4 +19,5 @@ commands = flake8 .
 exclude = .git, .tox, __pycache__, old, build, dist
 ignore =
     E501  # Lines are too long
+    E402  # Module level imports not at top of file
 import-order-style = pep8