Maintenance week updates (#206)

* Upgrade linting

* Replace setup.cfg with pyproject.toml
* Uninstall legacy linters and install current linters
* Add new linting commands to Makefile
* Add help command to Makefile
* Install pre-commit and add config yaml

* Changes requested by linters

* Updates to type hinting, code structure, datetime objects, unit tests, fixtures, and docstrings as requested by linters

* Add run-dev command to Makefile

* Update dependencies

* Updates based on discussion in PR #206

* Remove lambda-related Makefile commands
* Replace caplog.at_level with caplog.set_level in CLI test
* Remove coverage from Pipfile

.pre-commit-config.yaml

@@ -0,0 +1,29 @@
+default_language_version:
+  python: python3.11 # set for project python version
+repos:
+  - repo: local
+    hooks:
+      - id: black-apply
+        name: black-apply
+        entry: pipenv run black
+        language: system
+        pass_filenames: true
+        types: ["python"]
+      - id: mypy
+        name: mypy
+        entry: pipenv run mypy
+        language: system
+        pass_filenames: true
+        types: ["python"]
+        exclude: "tests/"
+      - id: ruff-apply
+        name: ruff-apply
+        entry: pipenv run ruff check --fix
+        language: system
+        pass_filenames: true
+        types: ["python"]
+      - id: safety
+        name: safety
+        entry: pipenv check
+        language: system
+        pass_filenames: false

Makefile

@@ -8,6 +8,10 @@ ECR_URL_DEV:=222053980223.dkr.ecr.us-east-1.amazonaws.com/alma-creditcardslips-d
 SHELL=/bin/bash
 DATETIME:=$(shell date -u +%Y%m%dT%H%M%SZ)
 
+help: ## Print this message
+	@awk 'BEGIN { FS = ":.*##"; print "Usage:  make <target>\n\nTargets:" } \
+/^[-_[:alpha:]]+:.?*##/ { printf "  %-15s%s\n", $$1, $$2 }' $(MAKEFILE_LIST)
+
 ### Dependency commands ###
 
 install: ## Install dependencies and CLI app
@@ -28,24 +32,30 @@ coveralls: test
 
 ### Code quality and safety commands ###
 
-lint: bandit black mypy pylama safety ## Run linting, code quality, and safety checks
-
-bandit:
-	pipenv run bandit -r ccslips
+# linting commands
+lint: black mypy ruff safety 
 
 black:
 	pipenv run black --check --diff .
 
 mypy:
-	pipenv run mypy ccslips
+	pipenv run mypy .
 
-pylama:
-	pipenv run pylama --options setup.cfg
+ruff:
+	pipenv run ruff check .
 
 safety:
 	pipenv check
 	pipenv verify
 
+# apply changes to resolve any linting errors
+lint-apply: black-apply ruff-apply
+
+black-apply: 
+	pipenv run black .
+
+ruff-apply: 
+	pipenv run ruff check --fix .
 
 ### Terraform-generated Developer Deploy Commands for Dev environment ###
 dist-dev: ## Build docker container (intended for developer-based manual build)
@@ -59,11 +69,6 @@ publish-dev: dist-dev ## Build, tag and push (intended for developer-based manua
 	docker push $(ECR_URL_DEV):latest
 	docker push $(ECR_URL_DEV):`git describe --always`
 
-### If this is a Lambda repo, uncomment the two lines below     ###
-# update-lambda-dev: ## Updates the lambda with whatever is the most recent image in the ecr (intended for developer-based manual update)
-#	aws lambda update-function-code --function-name $(FUNCTION_DEV) --image-uri $(ECR_URL_DEV):latest
-
-
 ### Terraform-generated manual shortcuts for deploying to Stage. This requires  ###
 ###   that ECR_NAME_STAGE, ECR_URL_STAGE, and FUNCTION_STAGE environment        ###
 ###   variables are set locally by the developer and that the developer has     ###
@@ -80,6 +85,5 @@ publish-stage: ## Only use in an emergency
 	docker push $(ECR_URL_STAGE):latest
 	docker push $(ECR_URL_STAGE):`git describe --always`
 
-### If this is a Lambda repo, uncomment the two lines below     ###
-# update-lambda-stage: ## Updates the lambda with whatever is the most recent image in the ecr (intended for developer-based manual update)
-#	aws lambda update-function-code --function-name $(FUNCTION_STAGE) --image-uri $(ECR_URL_STAGE):latest
+run-dev: # Run in dev against Alma sandbox
+	aws ecs run-task --cluster alma-integrations-creditcardslips-ecs-dev --task-definition alma-integrations-creditcardslips-ecs-dev --launch-type="FARGATE" --network-configuration '{ "awsvpcConfiguration": {"subnets": ["subnet-0488e4996ddc8365b", "subnet-022e9ea19f5f93e65"],"securityGroups": ["sg-095372030a26c7753"],"assignPublicIp": "DISABLED"}}'

Pipfile

@@ -10,17 +10,17 @@ requests = "*"
 sentry-sdk = "*"
 
 [dev-packages]
-bandit = "*"
 black = "*"
 boto3-stubs = "*"
-coverage = "*"
 coveralls = "*"
 freezegun = "*"
 moto = "*"
 mypy = "*"
-pylama = {extras = ["all"], version = "*"}
+pre-commit = "*"
 pytest = "*"
 requests-mock = "*"
+ruff = "*"
+safety = "*"
 types-requests = "*"
 
 [requires]

ccslips/alma.py

@@ -1,6 +1,6 @@
 import logging
 import time
-from typing import Generator, Optional
+from collections.abc import Generator
 from urllib.parse import urljoin
 
 import requests
@@ -39,7 +39,7 @@ def get_paged(
         self,
         endpoint: str,
         record_type: str,
-        params: Optional[dict] = None,
+        params: dict | None = None,
         limit: int = 100,
         _offset: int = 0,
         _records_retrieved: int = 0,
@@ -75,8 +75,7 @@ def get_paged(
         total_record_count = int(response.json()["total_record_count"])
         records = response.json().get(record_type, [])
         records_retrieved = _records_retrieved + len(records)
-        for record in records:
-            yield record
+        yield from records
         if records_retrieved < total_record_count:
             yield from self.get_paged(
                 endpoint,
@@ -88,10 +87,9 @@ def get_paged(
             )
 
     def get_brief_po_lines(
-        self, acquisition_method: Optional[str] = None
+        self, acquisition_method: str | None = None
     ) -> Generator[dict, None, None]:
-        """
-        Get brief PO line records, optionally filtered by acquisition_method.
+        """Get brief PO line records, optionally filtered by acquisition_method.
 
         The PO line records retrieved from this endpoint do not contain all of the PO
         line data and users may wish to retrieve the full PO line records with the
@@ -118,15 +116,13 @@ def get_full_po_line(self, po_line_id: str) -> dict:
 
     def get_full_po_lines(
         self,
-        acquisition_method: Optional[str] = None,
-        date: Optional[str] = None,
+        acquisition_method: str | None = None,
+        date: str | None = None,
     ) -> Generator[dict, None, None]:
         """Get full PO line records, optionally filtered by acquisition_method/date."""
         for line in self.get_brief_po_lines(acquisition_method):
             number = line["number"]
-            if date is None:
-                yield self.get_full_po_line(number)
-            elif line.get("created_date") == f"{date}Z":
+            if line.get("created_date") == f"{date}Z" or not date:
                 yield self.get_full_po_line(number)
 
     def get_fund_by_code(self, fund_code: str) -> dict:

ccslips/cli.py

@@ -1,8 +1,7 @@
+import datetime
 import logging
 import os
-from datetime import datetime, timedelta
 from time import perf_counter
-from typing import Optional
 
 import click
 
@@ -52,8 +51,8 @@ def main(
     ctx: click.Context,
     source_email: str,
     recipient_email: list[str],
-    date: Optional[str],
-    log_level: Optional[str],
+    date: str | None,
+    log_level: str | None,
 ) -> None:
     start_time = perf_counter()
     log_level = log_level or "INFO"
@@ -63,7 +62,9 @@ def main(
     logger.debug("Command called with options: %s", ctx.params)
 
     logger.info("Starting credit card slips process")
-    date = date or (datetime.today() - timedelta(days=2)).strftime("%Y-%m-%d")
+    date = date or (
+        datetime.datetime.now(tz=datetime.UTC) - datetime.timedelta(days=2)
+    ).strftime("%Y-%m-%d")
     credit_card_slips_data = process_po_lines(date)
     email_content = generate_credit_card_slips_html(credit_card_slips_data)
     email = Email()
@@ -90,5 +91,5 @@ def main(
         date,
         recipient_email,
         response["MessageId"],
-        str(timedelta(seconds=elapsed_time)),
+        str(datetime.timedelta(seconds=elapsed_time)),
     )

ccslips/config.py

@@ -6,9 +6,10 @@
 
 def configure_logger(logger: logging.Logger, log_level_string: str) -> str:
     if log_level_string.upper() not in logging.getLevelNamesMapping():
-        raise ValueError(f"'{log_level_string}' is not a valid Python logging level")
+        message = f"'{log_level_string}' is not a valid Python logging level"
+        raise ValueError(message)
     log_level = logging.getLevelName(log_level_string.upper())
-    if log_level < 20:
+    if log_level < 20:  # noqa: PLR2004
         logging.basicConfig(
             format="%(asctime)s %(levelname)s %(name)s.%(funcName)s() line %(lineno)d: "
             "%(message)s"

ccslips/email.py

@@ -1,6 +1,5 @@
 from email.message import EmailMessage
 from email.policy import EmailPolicy, default
-from typing import List, Optional
 
 import boto3
 
@@ -12,16 +11,16 @@ def __init__(self, policy: EmailPolicy = default) -> None:
         """Initialize Email instance."""
         super().__init__(policy)
 
-    def populate(  # noqa pylint R0913 too many arguments
+    def populate(
         self,
         from_address: str,
         to_addresses: str,
         subject: str,
-        attachments: Optional[List[dict]] = None,
-        body: Optional[str] = None,
-        bcc: Optional[str] = None,
-        cc: Optional[str] = None,  # noqa pylint C0103 not snake_case
-        reply_to: Optional[str] = None,
+        attachments: list[dict] | None = None,
+        body: str | None = None,
+        bcc: str | None = None,
+        cc: str | None = None,
+        reply_to: str | None = None,
     ) -> None:
         """Populate Email message with addresses and subject.
 
@@ -70,11 +69,10 @@ def send(self) -> dict[str, str]:
             destinations.extend(self["Cc"].split(","))
         if self["Bcc"]:
             destinations.extend(self["Bcc"].split(","))
-        response = ses.send_raw_email(
+        return ses.send_raw_email(
             Source=self["From"],
             Destinations=destinations,
             RawMessage={
                 "Data": self.as_bytes(),
             },
         )
-        return response

ccslips/polines.py

@@ -1,8 +1,8 @@
-import xml.etree.ElementTree as ET  # nosec B405
+from collections.abc import Generator, Iterator
 from copy import deepcopy
 from datetime import datetime
 from decimal import Decimal
-from typing import Generator, Iterator, Optional
+from xml.etree import ElementTree
 
 from ccslips.alma import AlmaClient
 
@@ -20,9 +20,11 @@ def extract_credit_card_slip_data(client: AlmaClient, po_line_record: dict) -> d
     The keys of the returned dict map to the appropriate element classes in the XML
     template used to generate a formatted slip.
     """
-    created_date = datetime.strptime(
-        po_line_record["created_date"], "%Y-%m-%dZ"
-    ).strftime("%y%m%d")
+    created_date = (
+        datetime.strptime(po_line_record["created_date"], "%Y-%m-%dZ")
+        .astimezone()
+        .strftime("%y%m%d")
+    )
     fund_distribution = po_line_record.get("fund_distribution", [])
     price = Decimal(po_line_record.get("price", {}).get("sum", "0.00"))
     title = po_line_record.get("resource_metadata", {}).get("title", "Unknown title")
@@ -47,15 +49,15 @@ def extract_credit_card_slip_data(client: AlmaClient, po_line_record: dict) -> d
     return po_line_data
 
 
-def get_cardholder_from_notes(notes: Optional[list[dict]]) -> str:
+def get_cardholder_from_notes(notes: list[dict] | None) -> str:
     """Get first note that begins with 'CC-' from a PO line record notes field."""
     if notes:
         for note in [n for n in notes if n.get("note_text", "").startswith("CC-")]:
             return note["note_text"][3:]
     return "No cardholder note found"
 
 
-def get_quantity_from_locations(locations: Optional[list[dict]]) -> str:
+def get_quantity_from_locations(locations: list[dict] | None) -> str:
     """Get the total quantity of items associated with PO line locations.
 
     This function adds the quantities from each location in the PO line. This is an
@@ -87,53 +89,51 @@ def get_total_price_from_fund_distribution(
     )
 
 
-def get_account_data(
-    client: AlmaClient, fund_distribution: list[dict]
-) -> dict[str, str]:
+def get_account_data(client: AlmaClient, fund_distribution: list[dict]) -> dict[str, str]:
     """Get account information needed for a credit card slip.
 
     If the fund_distribution is empty, returns a single account with default text.
     Otherwise returns up to two accounts with their associated account numbers.
     """
     result = {"account_1": "No fund code found"}
     for count, fund in enumerate(fund_distribution, start=1):
-        if count == 3:
+        if count == 3:  # noqa: PLR2004
             break
         if account_number := get_account_number_from_fund(client, fund):
             result[f"account_{count}"] = account_number
     return result
 
 
-def get_account_number_from_fund(client: AlmaClient, fund: dict) -> Optional[str]:
+def get_account_number_from_fund(client: AlmaClient, fund: dict) -> str | None:
     """Get account number for a given fund.
 
     Returns None if fund has no fund code, fund record cannot be retrieved via fund
     code, or fund record does not contain an external_id field value.
     """
     account = None
-    if fund_code := fund.get("fund_code", {}).get("value"):
+    if fund_code := fund.get("fund_code", {}).get("value"):  # noqa: SIM102
         if fund_records := client.get_fund_by_code(fund_code).get("fund"):
             account = fund_records[0].get("external_id")
     return account
 
 
 def generate_credit_card_slips_html(po_line_data: Iterator[dict]) -> str:
     """Create credit card slips HTML from a set of credit card slip data."""
-    template_tree = ET.parse("config/credit_card_slip_template.xml")  # nosec B314
+    template_tree = ElementTree.parse(  # noqa: S314
+        "config/credit_card_slip_template.xml"
+    )
     xml_template = template_tree.getroot()
-    output = ET.fromstring("<html></html>")  # nosec B314
+    output = ElementTree.fromstring("<html></html>")  # noqa: S314
     for line in po_line_data:
-        output.append(
-            populate_credit_card_slip_xml_fields(deepcopy(xml_template), line)
-        )
+        output.append(populate_credit_card_slip_xml_fields(deepcopy(xml_template), line))
     if len(output) == 0:
         return "<html><p>No credit card orders on this date</p></html>"
-    return ET.tostring(output, encoding="unicode", method="xml")
+    return ElementTree.tostring(output, encoding="unicode", method="xml")
 
 
 def populate_credit_card_slip_xml_fields(
-    credit_card_slip_xml_template: ET.Element, credit_card_slip_data: dict
-) -> ET.Element:
+    credit_card_slip_xml_template: ElementTree.Element, credit_card_slip_data: dict
+) -> ElementTree.Element:
     """Populate credit card slip XML template with data extracted from a PO line.
 
     The credit_card_slip_data keys must correspond to their associated element classes