Repository: RockefellerArchiveCenter/DACSspace

Issue:
Rethink handling of AS configs
**Is your feature request related to a problem? Please describe.**
The current implementation of this library asks for a config file with ArchivesSpace credentials. This works with a standalone script, but not with an installable package.

**Describe the solution you'd like**
Rethink how we provide AS configs to DACSspace. A couple of (non-mutually-exclusive) options:
- Allow users to point to a config file (if we're going to do this we should consider whether it makes sense to just move all the configs to a file
- Add additional arguments for AS credentials
- Look at how ASnake handles configuration and see if there's an approach we can piggyback on there.

Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index 630d230..34a30bb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,4 @@
-local_settings.cfg
+as_config.cfg
 log.txt
 *.csv
 *__pycache__
diff --git a/README.md b/README.md
index 35f3965..3649ae3 100644
--- a/README.md
+++ b/README.md
@@ -24,29 +24,23 @@ Download and install [Python](https://www.python.org/downloads/)
 
 * If you are using Windows, add Python to your [PATH variable](https://docs.python.org/2/using/windows.html)
 
-Download or [clone](https://docs.github.com/en/repositories/creating-and-managing-repositories/cloning-a-repository) this repository 
+Download or [clone](https://docs.github.com/en/repositories/creating-and-managing-repositories/cloning-a-repository) this repository
 
 Install requirements from within the main DACSspace directory: ```pip install -r requirements.txt```
 
 ## Setup
 
-Run `setup.py` to create a configuration file named `local_settings.cfg` which will allow you to connect to your ArchivesSpace instance. You will need:
-* BaseURL of your ArchivesSpace instance
-* Repository ID for your ArchivesSpace installation
-* ArchivesSpace username and password
+Create a file to hold your ArchivesSpace credentials. This file should contain:
+* The base URL of your ArchivesSpace instance
+* A Repository ID for your ArchivesSpace installation
+* An ArchivesSpace username and associated password
+
+The easiest way to do this is to rename `as_config.example` to `as_config.cfg`
+and update it with your values.
+
+By default, DACSspace expects this file to be named `as_config.cfg`, but you can
+pass a different filepath via the `as_config` command-line argument.  
 
-Your configuration file will look something like this:
-```
-[ArchivesSpace]
-baseURL: #BaseURL of your ArchivesSpace instance
-repository: #Repository ID of your ArchivesSpace installation
-user: #ArchivesSpace username
-password: #ArchivesSpace password for the username used above
-
-[Destinations]
-directory: #Directory where you would like to save the csv file
-filename: dacs_singlelevel_report.csv
-```
 
 ## Usage
 
diff --git a/dacsspace/local_settings.example b/as_config.example
similarity index 100%
rename from dacsspace/local_settings.example
rename to as_config.example
diff --git a/dacsspace/client.py b/dacsspace/client.py
index c86d558..b83eae7 100644
--- a/dacsspace/client.py
+++ b/dacsspace/client.py
@@ -1,5 +1,4 @@
 from configparser import ConfigParser
-from os.path import isfile
 
 from asnake.aspace import ASpace
 
@@ -11,13 +10,9 @@ class ASnakeConfigError(Exception):
 class ArchivesSpaceClient:
     """Handles communication with ArchivesSpace."""
 
-    def __init__(self):
-        config_filepath = "dacsspace/local_settings.cfg"
-        if not isfile("dacsspace/local_settings.cfg"):
-            raise IOError(
-                "Could not find a configuration file at {}".format(config_filepath))
+    def __init__(self, as_config):
         config = ConfigParser()
-        config.read(config_filepath)
+        config.read(as_config)
         self.aspace = ASpace(baseurl=config.get('ArchivesSpace', 'baseurl'),
                              username=config.get('ArchivesSpace', 'user'),
                              password=config.get('ArchivesSpace', 'password'))
diff --git a/dacsspace/command_line.py b/dacsspace/command_line.py
index 30c7472..d96267a 100644
--- a/dacsspace/command_line.py
+++ b/dacsspace/command_line.py
@@ -11,6 +11,11 @@ def main():
         'csv_filepath',
         help='Filepath for results report (CSV format)',
         type=str)
+    parser.add_argument(
+        '--as_config',
+        help='Filepath for ArchivesSpace configuration file',
+        typ=str,
+        default='as_config.cfg')
     parser.add_argument(
         '--published_only',
         help='Fetches only published records from AS',
@@ -32,7 +37,7 @@ def main():
         default=None)
     args = parser.parse_args()
 
-    DACSspace(args.csv_filepath).run(
+    DACSspace(args.as_config, args.csv_filepath).run(
         args.published_only,
         args.invalid_only,
         args.schema_identifier,
diff --git a/dacsspace/dacsspace.py b/dacsspace/dacsspace.py
index fd7454e..0488dc1 100644
--- a/dacsspace/dacsspace.py
+++ b/dacsspace/dacsspace.py
@@ -1,4 +1,5 @@
 import re
+from os.path import isfile
 
 from .client import ArchivesSpaceClient
 from .reporter import CSVReporter
@@ -8,7 +9,7 @@ from .validator import Validator
 class DACSspace:
     """Base DACSspace class. Fetches data from AS, validates and reports results."""
 
-    def __init__(self, csv_filepath):
+    def __init__(self, as_config, csv_filepath):
         """Checks csv filepath to make sure it has the proper extension and characters."""
         if not csv_filepath.endswith(".csv"):
             raise ValueError("File must have .csv extension")
@@ -16,10 +17,14 @@ class DACSspace:
             raise ValueError(
                 'File name cannot contain the following characters: * ? : " < > | ')
         self.csv_filepath = csv_filepath
+        if not isfile(as_config):
+            raise IOError(
+                "Could not find an ArchivesSpace configuration file at {}".format(as_config))
+        self.as_config = as_config
 
     def run(self, published_only, invalid_only,
             schema_identifier, schema_filepath):
-        client = ArchivesSpaceClient()
+        client = ArchivesSpaceClient(self.as_config)
         validator = Validator(schema_identifier, schema_filepath)
         reporter = CSVReporter(self.csv_filepath)
         data = client.get_resources(published_only)


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index 8bcd7d4..59059bb 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -8,25 +8,20 @@ from asnake.client.web_client import ASnakeAuthError
 
 from dacsspace.client import ArchivesSpaceClient, ASnakeConfigError
 
-CONFIG_FILEPATH = "dacsspace/local_settings.cfg"
+CONFIG_FILEPATH = "as_config.cfg"
 
 
 class ArchivesSpaceClientTests(unittest.TestCase):
     def setUp(self):
         """Move existing config file and replace with sample config."""
         if os.path.isfile(CONFIG_FILEPATH):
-            shutil.move(
-                CONFIG_FILEPATH,
-                "dacsspace/local_settings.old")
-        shutil.copy(
-            "dacsspace/local_settings.example",
-            CONFIG_FILEPATH)
+            shutil.move(CONFIG_FILEPATH, "as_config.old")
+        shutil.copy("as_config.example", CONFIG_FILEPATH)
 
     @patch("requests.Session.get")
     @patch("requests.Session.post")
     def test_config_file(self, mock_post, mock_get):
         """Asserts that configuration files are correctly handled:
-            - Missing configuration files cause an exception to be raised.
             - Configuration files without all the necessary values cause an exception to be raised.
             - Valid configuration file allows for successful instantiation of ArchivesSpaceClient class.
         """
@@ -37,7 +32,7 @@ class ArchivesSpaceClientTests(unittest.TestCase):
         mock_get.return_value.text = "v3.0.2"
 
         # Valid config file
-        ArchivesSpaceClient()
+        ArchivesSpaceClient(CONFIG_FILEPATH)
 
         # remove baseurl from ArchivesSpace section
         config = configparser.ConfigParser()
@@ -48,7 +43,7 @@ class ArchivesSpaceClientTests(unittest.TestCase):
 
         # Configuration file missing necessary options
         with self.assertRaises(configparser.NoOptionError) as err:
-            ArchivesSpaceClient()
+            ArchivesSpaceClient(CONFIG_FILEPATH)
         self.assertEqual(str(err.exception),
                          "No option 'baseurl' in section: 'ArchivesSpace'")
 
@@ -61,18 +56,9 @@ class ArchivesSpaceClientTests(unittest.TestCase):
 
         # Configuration file missing necessary section
         with self.assertRaises(configparser.NoSectionError) as err:
-            ArchivesSpaceClient()
+            ArchivesSpaceClient(CONFIG_FILEPATH)
         self.assertEqual(str(err.exception), "No section: 'ArchivesSpace'")
 
-        # delete file
-        os.remove(CONFIG_FILEPATH)
-
-        # Missing configuration file
-        with self.assertRaises(IOError) as err:
-            ArchivesSpaceClient()
-        self.assertEqual(str(err.exception),
-                         "Could not find a configuration file at dacsspace/local_settings.cfg")
-
     @patch("requests.Session.get")
     @patch("requests.Session.post")
     def test_connection(self, mock_post, mock_get):
@@ -85,14 +71,14 @@ class ArchivesSpaceClientTests(unittest.TestCase):
         # Incorrect authentication credentials
         mock_post.return_value.status_code = 403
         with self.assertRaises(ASnakeAuthError) as err:
-            ArchivesSpaceClient()
+            ArchivesSpaceClient(CONFIG_FILEPATH)
         self.assertEqual(str(err.exception),
                          "Failed to authorize ASnake with status: 403")
 
         # Incorrect base URL
         mock_post.return_value.status_code = 404
         with self.assertRaises(ASnakeAuthError) as err:
-            ArchivesSpaceClient()
+            ArchivesSpaceClient(CONFIG_FILEPATH)
         self.assertEqual(str(err.exception),
                          "Failed to authorize ASnake with status: 404")
 
@@ -104,7 +90,7 @@ class ArchivesSpaceClientTests(unittest.TestCase):
         mock_get.return_value.json.return_value = {
             'error': 'Repository not found'}
         with self.assertRaises(ASnakeConfigError) as err:
-            ArchivesSpaceClient()
+            ArchivesSpaceClient(CONFIG_FILEPATH)
         self.assertEqual(str(err.exception),
                          "Error getting repository: Repository not found")
 
@@ -114,7 +100,8 @@ class ArchivesSpaceClientTests(unittest.TestCase):
     def test_published_only(self, mock_authorize, mock_get, mock_search):
         """Asserts that the `published_only` flag is handled correctly."""
         mock_get.return_value.text = "v3.0.2"  # Allows ArchivesSpaceClient to instantiate
-        client = ArchivesSpaceClient()  # Instantiates ArchivesSpaceClient for testing
+        # Instantiates ArchivesSpaceClient for testing
+        client = ArchivesSpaceClient(CONFIG_FILEPATH)
 
         # Test search for only published resources
         list(client.get_resources(True))
@@ -134,7 +121,7 @@ class ArchivesSpaceClientTests(unittest.TestCase):
     def test_data(self, mock_authorize, mock_get, mock_search):
         """Asserts that individual resources are returned"""
         mock_get.return_value.text = "v3.0.2"
-        client = ArchivesSpaceClient()
+        client = ArchivesSpaceClient(CONFIG_FILEPATH)
 
         # create a mocked object which acts like an
         # `asnake.jsonmodel.JSONModel` object
@@ -149,7 +136,5 @@ class ArchivesSpaceClientTests(unittest.TestCase):
 
     def tearDown(self):
         """Replace sample config with existing config."""
-        if os.path.isfile("dacsspace/local_settings.old"):
-            shutil.move(
-                "dacsspace/local_settings.old",
-                CONFIG_FILEPATH)
+        if os.path.isfile("as_config.old"):
+            shutil.move("as_config.old", CONFIG_FILEPATH)
diff --git a/tests/test_dacsspace.py b/tests/test_dacsspace.py
index 4e52084..f20fef4 100644
--- a/tests/test_dacsspace.py
+++ b/tests/test_dacsspace.py
@@ -1,10 +1,19 @@
+import os
+import shutil
 from unittest import TestCase
 from unittest.mock import patch
 
 from dacsspace.dacsspace import DACSspace
 
+CONFIG_FILEPATH = "as_config.cfg"
+
 
 class TestDACSspace(TestCase):
+    def setUp(self):
+        """Move existing config file and replace with sample config."""
+        if os.path.isfile(CONFIG_FILEPATH):
+            shutil.move(CONFIG_FILEPATH, "as_config.old")
+        shutil.copy("as_config.example", CONFIG_FILEPATH)
 
     def test_csv_filepath(self):
         """Asserts that CSV filepath is handled as expected.
@@ -12,16 +21,24 @@ class TestDACSspace(TestCase):
         Filepaths are checked to ensure they end with the appropriate file
         extension (.csv) and don't contain any illegal characters.
         """
-        DACSspace("csv_filepath.csv")
+        DACSspace(CONFIG_FILEPATH, "csv_filepath.csv")
         with self.assertRaises(ValueError) as err:
-            DACSspace("my*file.csv")
+            DACSspace("as_config.example", "my*file.csv")
         self.assertEqual(str(err.exception),
                          'File name cannot contain the following characters: * ? : " < > | ')
         with self.assertRaises(ValueError) as err:
-            DACSspace("myfile")
+            DACSspace(CONFIG_FILEPATH, "myfile")
         self.assertEqual(str(err.exception),
                          "File must have .csv extension")
 
+    def test_as_config(self):
+        """Asserts missing files raise an exception."""
+        os.remove(CONFIG_FILEPATH)
+        with self.assertRaises(IOError) as err:
+            DACSspace(CONFIG_FILEPATH, "csv_filepath.csv")
+        self.assertEqual(str(err.exception),
+                         "Could not find an ArchivesSpace configuration file at as_config.cfg")
+
     @patch('dacsspace.client.ArchivesSpaceClient.__init__')
     @patch('dacsspace.client.ArchivesSpaceClient.get_resources')
     @patch('dacsspace.validator.Validator.__init__')
@@ -39,7 +56,7 @@ class TestDACSspace(TestCase):
                 ('test.csv', True, True, None, 'filepath/to/schema.json'),
                 ('testfile.csv', False, False, 'single_level_required', None),
                 ('file.csv', True, False, 'rac.json', None)]:
-            DACSspace(csv_filepath).run(
+            DACSspace("as_config.example", csv_filepath).run(
                 published_only,
                 invalid_only,
                 schema_identifier,
@@ -49,3 +66,8 @@ class TestDACSspace(TestCase):
             mock_reporter_init.assert_called_with(csv_filepath)
             mock_get_resources.assert_called_with(published_only)
             mock_write_report.assert_called_with([], invalid_only)
+
+    def tearDown(self):
+        """Replace sample config with existing config."""
+        if os.path.isfile("as_config.old"):
+            shutil.move("as_config.old", CONFIG_FILEPATH)