Merge branch 'qa' into production

.python-version

@@ -0,0 +1 @@
+3.9.16

CHANGELOG.md

@@ -1,4 +1,17 @@
 # Changelog
+## v1.1.2
+- Update config_helper to accept list environment variables
+
+## v1.1.0/v1.1.1
+- Add retries for empty responses in oauth2 client. This was added to address a known quirk in the Sierra API where this response is returned:
+```
+> GET / HTTP/1.1
+> Host: ilsstaff.nypl.org
+> User-Agent: curl/7.64.1
+> Accept: */*
+> 
+```
+- Due to an accidental deployment, v1.1.0 and v1.1.1 were both released but are identical
 
 ## v1.0.4 - 6/28/23
 - Enforce Kinesis stream 1000 records/second write limit

README.md

@@ -14,7 +14,7 @@ This package contains common Python utility classes and functions.
 * Making requests to the Oauth2 authenticated APIs such as NYPL Platform API and Sierra
 
 ## Functions
-* Reading a YAML config file and putting the contents in os.environ
+* Reading a YAML config file and putting the contents in os.environ -- see `config/sample.yaml` for an example of how the config file should be formatted
 * Creating a logger in the appropriate format
 * Obfuscating a value using bcrypt
 * Parsing/building Research Catalog identifiers
@@ -35,19 +35,19 @@ kinesis_client = KinesisClient(...)
 # Do not use any version below 1.0.0
 # All available optional dependencies can be found in pyproject.toml.
 # See the "Managing dependencies" section below for more details.
-nypl-py-utils[kinesis-client,config-helper]==1.0.4
+nypl-py-utils[kinesis-client,config-helper]==1.1.2
 ```
 
 ## Developing locally
 In order to use the local version of the package instead of the global version, use a virtual environment. To set up a virtual environment and install all the necessary dependencies, run:
 
 ```
-python3 -m venv testenv
-source testenv/bin/activate
+python3 -m venv .venv
+source .venv/bin/activate
 pip install --upgrade pip
 pip install .
 pip install '.[development]'
-deactivate && source testenv/bin/activate
+deactivate && source .venv/bin/activate
 ```
 
 ## Managing dependencies
@@ -57,11 +57,11 @@ When a new client or helper file is created, a new optional dependency set shoul
 
 The optional dependency sets also give the developer the option to manually list out the dependencies of the clients rather than relying upon what the package thinks is required, which can be beneficial in certain circumstances. For instance, AWS lambda functions come with `boto3` and `botocore` pre-installed, so it's not necessary to include these (rather hefty) dependencies in the lambda deployment package.
 
-### Troubleshooting
-#### Using PostgreSQLClient in an AWS Lambda
+## Troubleshooting
+### Using PostgreSQLClient in an AWS Lambda
 Because `psycopg` requires a statically linked version of the `libpq` library, the `PostgreSQLClient` cannot be installed as-is in an AWS Lambda function. Instead, it must be packaged as follows:
 ```bash
-pip install --target ./package nypl-py-utils[postgresql-client]==1.0.1
+pip install --target ./package nypl-py-utils[postgresql-client]==1.1.2
 
 pip install \
     --platform manylinux2014_x86_64 \
@@ -72,7 +72,7 @@ pip install \
     'psycopg[binary]'
 ```
 
-#### Using PostgreSQLClient locally
+### Using PostgreSQLClient locally
 If using the `PostgreSQLClient` produces the following error locally:
 ```
 ImportError: no pq wrapper available.

config/sample.yaml

@@ -0,0 +1,14 @@
+---
+PLAINTEXT_VARIABLES:
+    STRING_VAR: string-var
+    INT_VAR: 1
+    LIST_VAR:
+        - string-var2
+        - 2
+ENCRYPTED_VARIABLES:
+    ENCRYPTED_STRING_VAR: AQECAHh7ea2tyZ6phZgT4B9BDKwguhlFtRC6hgt+7HbmeFsrsgAAAGowaAYJKoZIhvcNAQcGoFswWQIBADBUBgkqhkiG9w0BBwEwHgYJYIZIAWUDBAEuMBEEDCvE8Pc8PiUEiCGpEAIBEIAnf8fz6YXH959A0ygrM4S95giFnwvp9dYFzp/2ViAIlD5GZ1S04vay
+    ENCRYPTED_INT_VAR: AQECAHh7ea2tyZ6phZgT4B9BDKwguhlFtRC6hgt+7HbmeFsrsgAAAF8wXQYJKoZIhvcNAQcGoFAwTgIBADBJBgkqhkiG9w0BBwEwHgYJYIZIAWUDBAEuMBEEDFQdg7ua7D8XH7UZGgIBEIAcpkIN6+56sbR3Vbk12NX2QDY28dnL8IWgVdnBRA==
+    ENCRYPTED_LIST_VAR:
+        - AQECAHh7ea2tyZ6phZgT4B9BDKwguhlFtRC6hgt+7HbmeFsrsgAAAGswaQYJKoZIhvcNAQcGoFwwWgIBADBVBgkqhkiG9w0BBwEwHgYJYIZIAWUDBAEuMBEEDMTe0jJyHxGaiy0PHQIBEIAo4+qpfJp/gfZqhl1GtN/q9ebn2isiVOn5QLK/fcUtWeG182jiKPdOFA==
+        - AQECAHh7ea2tyZ6phZgT4B9BDKwguhlFtRC6hgt+7HbmeFsrsgAAAF8wXQYJKoZIhvcNAQcGoFAwTgIBADBJBgkqhkiG9w0BBwEwHgYJYIZIAWUDBAEuMBEEDGsG/1m7nes884q8vQIBEIAc9eBDgUgVzVsK3lyebNmc09kGfP7Gzwm6ESJAiA==
+...

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "nypl_py_utils"
-version = "1.0.4"
+version = "1.1.2"
 authors = [
   { name="Aaron Friedman", email="[email protected]" },
 ]

src/nypl_py_utils/classes/oauth2_api_client.py

@@ -1,4 +1,6 @@
 import os
+from time import sleep
+from requests.models import Response
 from oauthlib.oauth2 import BackendApplicationClient, TokenExpiredError
 from requests_oauthlib import OAuth2Session
 from nypl_py_utils.functions.log_helper import create_log
@@ -7,11 +9,14 @@
 class Oauth2ApiClient:
     """
     Client for interacting with an Oauth2 authenticated API such as NYPL's
-    Platform API endpoints
+    Platform API endpoints. Note with_retries is a boolean flag which
+    determines if empty get requests will be retried 3 times or until
+    they are successful. This is to address a known issue with the Sierra
+    API where empty responses are returned intermittently.
     """
 
     def __init__(self, client_id=None, client_secret=None, base_url=None,
-                 token_url=None):
+                 token_url=None, with_retries=False):
         self.client_id = client_id \
             or os.environ.get('NYPL_API_CLIENT_ID', None)
         self.client_secret = client_secret \
@@ -25,11 +30,30 @@ def __init__(self, client_id=None, client_secret=None, base_url=None,
 
         self.logger = create_log('oauth2_api_client')
 
+        self.with_retries = with_retries
+
     def get(self, request_path, **kwargs):
         """
         Issue an HTTP GET on the given request_path
         """
-        return self._do_http_method('GET', request_path, **kwargs)
+        resp = self._do_http_method('GET', request_path, **kwargs)
+        if resp.json() is None and self.with_retries is True:
+            retries = \
+                kwargs.get('retries', 0) + 1
+            if retries < 3:
+                self.logger.warning(
+                    f'Retrying get request due to empty response from\
+                         Oauth2 Client using path: {request_path}. \
+                         Retry #{retries}')
+                sleep(pow(2, retries - 1))
+                kwargs['retries'] = retries
+                resp = self.get(request_path, **kwargs)
+            else:
+                resp = Response()
+                resp.message = 'Oauth2 Client: Request failed after 3 \
+                        empty responses received from Oauth2 Client'
+                resp.status_code = 500
+        return resp
 
     def post(self, request_path, json, **kwargs):
         """
@@ -79,7 +103,7 @@ def _do_http_method(self, method, request_path, **kwargs):
                     from None
 
             self._generate_access_token()
-            return self._do_http_method(method, request_path, **kwargs)
+            return self._do_http_method(method, request_path, **kwargs).json()
 
     def _create_oauth_client(self):
         """

src/nypl_py_utils/functions/config_helper.py

@@ -1,3 +1,4 @@
+import json
 import os
 import yaml
 
@@ -11,15 +12,17 @@ def load_env_file(run_type, file_string):
     """
     This method loads a YAML config file containing environment variables,
     decrypts whichever are encrypted, and puts them all into os.environ as
-    strings.
+    strings. For a YAML variable containing a list of values, the list is
+    exported into os.environ as a json string and should be loaded as such.
 
     It requires the YAML file to be split into a 'PLAINTEXT_VARIABLES' section
-    and an 'ENCRYPTED_VARIABLES' section.
+    and an 'ENCRYPTED_VARIABLES' section. See config/sample.yaml for an example
+    config file.
 
     Parameters
         ----------
         run_type: str
-            The name of the config file to use, e.g. 'devel'
+            The name of the config file to use, e.g. 'sample'
         file_string: str
             The path to the config files with the filename as a variable to be
             interpolated, e.g. 'config/{}.yaml'
@@ -43,11 +46,18 @@ def load_env_file(run_type, file_string):
 
     if env_dict:
         for key, value in env_dict.get('PLAINTEXT_VARIABLES', {}).items():
-            os.environ[key] = str(value)
+            if type(value) is list:
+                os.environ[key] = json.dumps(value)
+            else:
+                os.environ[key] = str(value)
 
         kms_client = KmsClient()
         for key, value in env_dict.get('ENCRYPTED_VARIABLES', {}).items():
-            os.environ[key] = kms_client.decrypt(value)
+            if type(value) is list:
+                decrypted_list = [kms_client.decrypt(v) for v in value]
+                os.environ[key] = json.dumps(decrypted_list)
+            else:
+                os.environ[key] = kms_client.decrypt(value)
         kms_client.close()
 
 

tests/test_config_helper.py

@@ -4,16 +4,24 @@
 from nypl_py_utils.functions.config_helper import (
     load_env_file, ConfigHelperError)
 
-_TEST_VARIABLE_NAMES = ['TEST_STRING', 'TEST_INT', 'TEST_ENCRYPTED_VARIABLE_1',
-                        'TEST_ENCRYPTED_VARIABLE_2']
+_TEST_VARIABLE_NAMES = [
+    'TEST_STRING', 'TEST_INT', 'TEST_LIST', 'TEST_ENCRYPTED_VARIABLE_1',
+    'TEST_ENCRYPTED_VARIABLE_2', 'TEST_ENCRYPTED_LIST']
+
 _TEST_CONFIG_CONTENTS = \
     '''---
 PLAINTEXT_VARIABLES:
     TEST_STRING: string-variable
     TEST_INT: 1
+    TEST_LIST:
+        - string-var
+        - 2
 ENCRYPTED_VARIABLES:
     TEST_ENCRYPTED_VARIABLE_1: test-encryption-1
     TEST_ENCRYPTED_VARIABLE_2: test-encryption-2
+    TEST_ENCRYPTED_LIST:
+        - test-encryption-3
+        - test-encryption-4
 ...'''
 
 
@@ -22,7 +30,8 @@ class TestConfigHelper:
     def test_load_env_file(self, mocker):
         mock_kms_client = mocker.MagicMock()
         mock_kms_client.decrypt.side_effect = [
-            'test-decryption-1', 'test-decryption-2']
+            'test-decryption-1', 'test-decryption-2', 'test-decryption-3',
+            'test-decryption-4']
         mocker.patch('nypl_py_utils.functions.config_helper.KmsClient',
                      return_value=mock_kms_client)
         mock_file_open = mocker.patch(
@@ -34,13 +43,17 @@ def test_load_env_file(self, mocker):
 
         mock_file_open.assert_called_once_with('test-path/test-env.yaml', 'r')
         mock_kms_client.decrypt.assert_has_calls([
-            mocker.call('test-encryption-1'), mocker.call('test-encryption-2')]
+            mocker.call('test-encryption-1'), mocker.call('test-encryption-2'),
+            mocker.call('test-encryption-3'), mocker.call('test-encryption-4')]
         )
         mock_kms_client.close.assert_called_once()
         assert os.environ['TEST_STRING'] == 'string-variable'
         assert os.environ['TEST_INT'] == '1'
+        assert os.environ['TEST_LIST'] == '["string-var", 2]'
         assert os.environ['TEST_ENCRYPTED_VARIABLE_1'] == 'test-decryption-1'
         assert os.environ['TEST_ENCRYPTED_VARIABLE_2'] == 'test-decryption-2'
+        assert os.environ['TEST_ENCRYPTED_LIST'] == \
+            '["test-decryption-3", "test-decryption-4"]'
 
         for key in _TEST_VARIABLE_NAMES:
             if key in os.environ:

tests/test_oauth2_api_client.py

@@ -20,6 +20,18 @@
 TOKEN_URL = 'https://oauth.example.com/oauth/token'
 
 
+class MockEmptyResponse:
+    def __init__(self, empty, status_code=None):
+        self.status_code = status_code
+        self.empty = empty
+
+    def json(self):
+        if self.empty:
+            return None
+        else:
+            return 'success'
+
+
 class TestOauth2ApiClient:
 
     @pytest.fixture
@@ -36,6 +48,15 @@ def test_instance(self, requests_mock):
                                client_secret='clientsecret'
                                )
 
+    @pytest.fixture
+    def test_instance_with_retries(self, requests_mock):
+        return Oauth2ApiClient(base_url=BASE_URL,
+                               token_url=TOKEN_URL,
+                               client_id='clientid',
+                               client_secret='clientsecret',
+                               with_retries=True
+                               )
+
     def test_uses_env_vars(self):
         env = {
             'NYPL_API_CLIENT_ID': 'env client id',
@@ -136,3 +157,24 @@ def test_token_refresh_failure_raises_error(
             test_instance._do_http_method('GET', 'foo')
         # Expect 1 initial token fetch, plus 3 retries:
         assert len(token_server_post.request_history) == 4
+
+    def test_http_retry_fail(self, requests_mock, test_instance_with_retries,
+                             token_server_post, mocker):
+        mocker.patch.object(test_instance_with_retries, '_do_http_method',
+                            return_value=MockEmptyResponse(empty=True))
+        get_spy = mocker.spy(test_instance_with_retries, 'get')
+        resp = test_instance_with_retries.get('spaghetti')
+        assert get_spy.call_count == 3
+        assert resp.status_code == 500
+
+    def test_http_retry_success(self, requests_mock,
+                                test_instance_with_retries,
+                                token_server_post, mocker):
+        mocker.patch.object(test_instance_with_retries, '_do_http_method',
+                            side_effect=[MockEmptyResponse(empty=True),
+                                         MockEmptyResponse(empty=False,
+                                                           status_code=200)])
+        get_spy = mocker.spy(test_instance_with_retries, 'get')
+        resp = test_instance_with_retries.get('spaghetti')
+        assert get_spy.call_count == 2
+        assert resp.json() == 'success'