Merge branch 'main' into qa - v1.0.0

Author: aaronfriedman6

.github/workflows/run-unit-tests.yml

@@ -28,7 +28,7 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           pip install .
-          pip install '.[tests]'
+          pip install '.[development]'
 
       - name: Run linter and test suite
         run: |

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v1.0.0 - 3/22/23
+- Improve Oauth2ApiClient token refresh and method responses
+- Create separate PostgreSQLClient and PostgreSQLPoolClient classes
+- Update PostgreSQL and MySQL clients to accept write queries implicitly
+- Update RedshiftClient to ensure SSL is being used
+- Separate dependencies to slim down package installation
+
 ## v0.0.7 - 3/1/23
 - Added Oauth2ApiClient for oauth2 authenticated calls to our Platform API and Sierra
 - Set PostgreSQL connection pool to have a default pool size minimum of 0
@@ -13,17 +20,14 @@
 - Updated README with deployment information
 
 ## v0.0.3 - 2/10/23
-
 - Added GitHub Actions workflow for deploying to production
 - Switched PostgreSQLClient to use connection pooling
 
 ## v0.0.2 - 2/6/23
-
 - Added CODEOWNERS
 - Added GitHub Actions workflows for running tests and deploying to QA
 - Added tests for helper functions
 - Updated Avro encoder to avoid dependency on pandas
 
 ## v0.0.1 - 1/26/23
-
 Initial version. Includes the `avro_encoder`, `kinesis_client`, `mysql_client`, `postgresql_client`, `redshift_client`, and `s3_client` classes as well as the `config_helper`, `kms_helper`, `log_helper`, and `obfuscation_helper` functions.

README.md

@@ -8,9 +8,10 @@ This package contains common Python utility classes and functions.
 * Decrypting values with KMS
 * Encoding and decoding records using a given Avro schema
 * Connecting to and querying a MySQL database
+* Connecting to and querying a PostgreSQL database
 * Connecting to and querying a PostgreSQL database using a connection pool
 * Connecting to and querying Redshift
-* Making requests to the Oauth2 authenticated APIS such as NYPL Platform API and Sierra
+* 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
@@ -25,11 +26,16 @@ python3 -m venv testenv
 source testenv/bin/activate
 pip install --upgrade pip
 pip install .
-pip install '.[tests]'
+pip install '.[development]'
 deactivate && source testenv/bin/activate
 ```
 
-Add any new dependencies required by code in the `nypl_py_utils` directory to the `dependencies` section of `pyproject.toml`. Add dependencies only required by code in the `tests` directory to the `[project.optional-dependencies]` section.
+## Managing dependencies
+In order to prevent dependency bloat, this package has no required dependencies. Instead, each class and helper file has its own optional dependency set. For instance, if an app needs to use the KMS client and the obfuscation helper, it should add `nypl-py-utils[kms-client, obfuscation-helper]` to the app's requirements. This way, only the required dependencies are installed.
+
+When a new client or helper file is created, a new optional dependency set should be added to `pyproject.toml`. The `development` dependency set, which includes all the dependencies required by all of the classes and tests, should also be updated.
+
+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
 If running `main.py` in this virtual environment produces the following error:

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "nypl_py_utils"
-version = "0.0.7"
+version = "1.0.0"
 authors = [
   { name="Aaron Friedman", email="aaronfriedman@nypl.org" },
 ]
@@ -16,30 +16,59 @@ classifiers = [
     "License :: OSI Approved :: MIT License",
     "Operating System :: OS Independent",
 ]
-dependencies = [
+dependencies = []
+
+[project.urls]
+"Homepage" = "https://github.com/NYPL/python-utils"
+"Bug Tracker" = "https://github.com/NYPL/python-utils/issues"
+
+[project.optional-dependencies]
+avro-encoder = [
     "avro>=1.11.1",
-    "bcrypt>=4.0.1",
+    "requests>=2.28.1"
+]
+kinesis-client = [
     "boto3>=1.26.5",
-    "botocore>=1.29.5",
-    "mysql-connector-python>=8.0.32",
+    "botocore>=1.29.5"
+]
+kms-client = [
+    "boto3>=1.26.5",
+    "botocore>=1.29.5"
+]
+mysql-client = [
+    "mysql-connector-python>=8.0.32"
+]
+oauth2-api-client = [
     "oauthlib>=3.2.2",
-    "psycopg[binary,pool]>=3.1.6",
-    "PyYAML>=6.0",
-    "redshift-connector>=2.0.909",
-    "requests>=2.28.1",
     "requests_oauthlib>=1.3.1"
 ]
-
-[project.optional-dependencies]
-tests = [
+postgresql-client = [
+    "psycopg[binary]>=3.1.6"
+]
+postgresql-pool-client = [
+    "psycopg[binary,pool]>=3.1.6"
+]
+redshift-client = [
+    "botocore>=1.29.5",
+    "redshift-connector>=2.0.909"
+]
+s3-client = [
+    "boto3>=1.26.5",
+    "botocore>=1.29.5"
+]
+config-helper = [
+    "nypl_py_utils[kms-client]",
+    "PyYAML>=6.0"
+]
+obfuscation-helper = [
+    "bcrypt>=4.0.1"
+]
+development = [
+    "nypl_py_utils[avro-encoder,kinesis-client,kms-client,mysql-client,oauth2-api-client,postgresql-client,postgresql-pool-client,redshift-client,s3-client,config-helper,obfuscation-helper]",
     "flake8>=6.0.0",
     "freezegun>=1.2.2",
     "mock>=4.0.3",
     "pytest>=7.2.0",
     "pytest-mock>=3.10.0",
     "requests-mock>=1.10.0"
 ]
-
-[project.urls]
-"Homepage" = "https://github.com/NYPL/python-utils"
-"Bug Tracker" = "https://github.com/NYPL/python-utils/issues"

src/nypl_py_utils/__init__.py

@@ -1,8 +0,0 @@
-from .classes.avro_encoder import AvroEncoder, AvroEncoderError  # noqa
-from .classes.kinesis_client import KinesisClient, KinesisClientError  # noqa
-from .classes.kms_client import KmsClient, KmsClientError # noqa
-from .classes.mysql_client import MySQLClient, MySQLClientError  # noqa
-from .classes.oauth2_api_client import Oauth2ApiClient  # noqa
-from .classes.postgresql_client import PostgreSQLClient, PostgreSQLClientError  # noqa
-from .classes.redshift_client import RedshiftClient, RedshiftClientError  # noqa
-from .classes.s3_client import S3Client, S3ClientError  # noqa

src/nypl_py_utils/classes/mysql_client.py

@@ -15,16 +15,28 @@ def __init__(self, host, port, database, user, password):
         self.user = user
         self.password = password
 
-    def connect(self):
-        """Connects to a MySQL database using the given credentials"""
+    def connect(self, **kwargs):
+        """
+        Connects to a MySQL database using the given credentials.
+
+        Keyword args can be passed into the connection to set certain options.
+        All possible arguments can be found here:
+        https://dev.mysql.com/doc/connector-python/en/connector-python-connectargs.html.
+
+        Common arguments include:
+            autocommit: bool
+                Whether to automatically commit each query rather than running
+                them as part of a transaction. By default False.
+        """
         self.logger.info('Connecting to {} database'.format(self.database))
         try:
             self.conn = mysql.connector.connect(
                 host=self.host,
                 port=self.port,
                 database=self.database,
                 user=self.user,
-                password=self.password)
+                password=self.password,
+                **kwargs)
         except mysql.connector.Error as e:
             self.logger.error(
                 'Error connecting to {name} database: {error}'.format(
@@ -33,37 +45,38 @@ def connect(self):
                 'Error connecting to {name} database: {error}'.format(
                     name=self.database, error=e)) from None
 
-    def execute_query(self, query, is_write_query=False, query_params=None,
-                      dictionary=False):
+    def execute_query(self, query, query_params=None, **kwargs):
         """
         Executes an arbitrary query against the given database connection.
 
         Parameters
         ----------
         query: str
             The query to execute
-        is_write_query: bool, optional
-            Whether or not the query is writing to the database, in which case
-            the transaction needs to be committed and None should be returned
         query_params: sequence, optional
             The values to be used in a parameterized query
-        dictionary: bool, optional
-            Whether the data will be returned as a dictionary. Defaults to
-            False, which means the data is returned as a list of tuples.
+        kwargs:
+            All possible arguments can be found here:
+            https://dev.mysql.com/doc/connector-python/en/connector-python-api-mysqlconnection-cursor.html.
+
+            Common arguments include:
+                dictionary: bool
+                    Whether the data will be returned as a dictionary. Defaults
+                    to False, meaning the data is returned as a list of tuples.
 
         Returns
         -------
         None or sequence
-            None if is_write_query is True. A list of either tuples or
-            dictionaries (based on the dictionary input) if is_write_query is
-            False.
+            None if the cursor has nothing to return. A list of either tuples
+            or dictionaries (based on the dictionary input) if there's
+            something to return (even if the result set is empty).
         """
         self.logger.info('Querying {} database'.format(self.database))
         self.logger.debug('Executing query {}'.format(query))
         try:
-            cursor = self.conn.cursor(dictionary=dictionary)
+            cursor = self.conn.cursor(**kwargs)
             cursor.execute(query, query_params)
-            if is_write_query:
+            if cursor.description is None:
                 self.conn.commit()
                 return None
             else:

src/nypl_py_utils/classes/oauth2_api_client.py

@@ -21,8 +21,7 @@ def __init__(self, client_id=None, client_secret=None, base_url=None,
         self.base_url = base_url \
             or os.environ.get('NYPL_API_BASE_URL', None)
 
-        self.client = None
-        self.token = None
+        self.oauth_client = None
 
         self.logger = create_log('oauth2_api_client')
 
@@ -56,39 +55,52 @@ def _do_http_method(self, method, request_path, **kwargs):
         """
         Issue an HTTP method call on on the given request_path
         """
-        if not self.client:
+        if not self.oauth_client:
             self._create_oauth_client()
 
         url = f'{self.base_url}/{request_path}'
         self.logger.debug(f'{method} {url}')
 
         try:
-            return self.oauth_client.request(method, url, **kwargs).json()
-        except TokenExpiredError as e:
-            self.logger.debug(f'TokenExpiredError encountered: {e}')
-            self._generate_access_token()
+            # Build kwargs cleaned of local variables:
+            kwargs_cleaned = {k: kwargs[k] for k in kwargs
+                              if not k.startswith('_do_http_method_')}
+            resp = self.oauth_client.request(method, url, **kwargs_cleaned)
+            resp.raise_for_status()
+            return resp
+        except TokenExpiredError:
+            self.logger.debug('TokenExpiredError encountered')
+
+            # Raise error after 3 successive token refreshes
+            kwargs['_do_http_method_token_refreshes'] = \
+                kwargs.get('_do_http_method_token_refreshes', 0) + 1
+            if kwargs['_do_http_method_token_refreshes'] > 3:
+                raise Oauth2ApiClientError('Exhausted token refreshes') \
+                    from None
 
+            self._generate_access_token()
             return self._do_http_method(method, request_path, **kwargs)
-        except TimeoutError as e:
-            self.logger.error(f'TimeoutError encountered: {e}')
-            return {}
 
     def _create_oauth_client(self):
         """
         Creates an authenticated a OAuth2Session instance for later requests
         """
+        client = BackendApplicationClient(client_id=self.client_id)
+        self.oauth_client = OAuth2Session(client=client)
         self._generate_access_token()
-        self.oauth_client = OAuth2Session(self.client_id, token=self.token)
 
     def _generate_access_token(self):
         """
         Fetch and store a fresh token
         """
-        client = BackendApplicationClient(client_id=self.client_id)
-        oauth = OAuth2Session(client=client)
         self.logger.debug(f'Refreshing token via @{self.token_url}')
-        self.token = oauth.fetch_token(
+        self.oauth_client.fetch_token(
             token_url=self.token_url,
             client_id=self.client_id,
             client_secret=self.client_secret
         )
+
+
+class Oauth2ApiClientError(Exception):
+    def __init__(self, message=None):
+        self.message = message