Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,30 +1,29 @@
#### What does this PR do?
A few sentences describing the overall goals of the pull request's commits.
Why are we making these changes? Is there more work to be done to fully
achieve these goals?
### Purpose and background context
Describe the overall purpose of the PR changes and any useful background context.

#### Helpful background context
Describe any additional context beyond what the PR accomplishes if it is likely
to be useful to a reviewer.

Delete this section if it isn't applicable to the PR.

#### How can a reviewer manually see the effects of these changes?
### How can a reviewer manually see the effects of these changes?
Explain how to see the proposed changes in the application if possible.

Delete this section if it isn't applicable to the PR.

#### Includes new or updated dependencies?
### Includes new or updated dependencies?
YES | NO

#### Developer
- [ ] README is updated to reflect all changes as needed
### Changes expectations for external applications?
YES | NO

### What are the relevant tickets?
- Include links to Jira Software and/or Jira Service Management tickets here.

### Developer
- [ ] All new ENV is documented in README
- [ ] All new ENV has been added to staging and production environments
- [ ] All related Jira tickets are linked in commit message(s)
- [ ] Stakeholder approval has been confirmed (or is not needed)

#### Code Reviewer
- [ ] The commit message is clear and follows our guidelines
(not just this pull request message)
### Code Reviewer(s)
- [ ] The commit message is clear and follows our guidelines (not just this PR message)
- [ ] There are appropriate tests covering any new functionality
- [ ] The documentation has been updated to reflect all changes or is not needed
- [ ] The changes have been verified
- [ ] The provided documentation is sufficient for understanding any new functionality introduced
- [ ] Any manual tests have been performed and verified
- [ ] New dependencies are appropriate or there were no changes
2 changes: 1 addition & 1 deletion .python-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.11.2
3.12
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM python:3.11-slim
FROM python:3.12-slim

# Set path for Oracle client libraries
ENV LD_LIBRARY_PATH /opt/lib/
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ S3_BUCKET:=shared-files-$(shell aws sts get-caller-identity --query "Account" --
ORACLE_ZIP:=instantclient-basiclite-linux.x64-21.9.0.0.0dbru.zip
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 python dependencies
Expand Down
7 changes: 3 additions & 4 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,18 @@ pyftpdlib = "*"
pyopenssl = "*"
ruff = "*"
types-setuptools = "*"

safety = "*"

[packages]
boto3 = "*"
click = "*"
lxml = "*"
oracledb = "*"
SQLAlchemy = "*"

sentry-sdk = "*"

[requires]
python_version = "3.11"
python_version = "3.12"

[scripts]
carbon = "python -c \"from carbon.cli import main; main()\""
carbon = "python -c \"from carbon.cli import main; main()\""
1,692 changes: 1,022 additions & 670 deletions Pipfile.lock

Large diffs are not rendered by default.

40 changes: 16 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,29 +136,21 @@ The password for the Data Warehouse is updated each year. To verify that the upd
* Verify that the following log is included:
> Successfully connected to the Data Warehouse: \<VERSION NUMBER\>

## Required ENV

```
WORKSPACE= "dev"

# type of feed, either "people" or "articles"
FEED_TYPE="people"

# JSON formatted string of key/value pairs for the MIT Data Warehouse connection
DATAWAREHOUSE_CLOUDCONNECTOR_JSON='{"USER": "<VALID_DATAWAREHOUSE_USERNAME>", "PASSWORD": "<VALID_DATAWAREHOUSE_PASSWORD>", "HOST": "<VALID_DATAWAREHOUSE_HOST>", "PORT": "<VALID_DATAWAREHOUSE_PORT>", "PATH": "<VALID_DATAWAREHOUSE_ORACLE_SID>", "CONNECTION_STRING": "<VALID_DATAWAREHOUSE_CONNECTION_STRING>"}'

# A JSON formatted string of key/value pairs for connecting to the Symplectic Elements FTP server
SYMPLECTIC_FTP_JSON='{"SYMPLECTIC_FTP_HOST": "<VALID_ELEMENTS_FTP_HOST>", "SYMPLECTIC_FTP_PORT": "<VALID_ELEMENTS_FTP_PORT>", "SYMPLECTIC_FTP_USER": "<VALID_ELEMENTS_FTP_USER>", "SYMPLECTIC_FTP_PASS": "<VALID_ELEMENTS_FTP_PASSWORD>"}'

# full XML file path that is uploaded to the Symplectic Elements FTP server
SYMPLECTIC_FTP_PATH="<FTP_FILE_DIRECTORY>/people.xml"

# SNS topic ARN used for sending email notifications.
SNS_TOPIC="<VALID_SNS_TOPIC_ARN>"
## Environment Variables

### Required
```shell
WORKSPACE="dev" # Set to `dev` for local development, this will be set to `stage` and `prod` in those environments by Terraform.
FEED_TYPE="people" # Type of feed, either "people" or "articles".
DATAWAREHOUSE_CLOUDCONNECTOR_JSON='{"USER": "<VALID_DATAWAREHOUSE_USERNAME>", "PASSWORD": "<VALID_DATAWAREHOUSE_PASSWORD>", "HOST": "<VALID_DATAWAREHOUSE_HOST>", "PORT": "<VALID_DATAWAREHOUSE_PORT>", "PATH": "<VALID_DATAWAREHOUSE_ORACLE_SID>", "CONNECTION_STRING": "<VALID_DATAWAREHOUSE_CONNECTION_STRING>"}' # JSON formatted string of key/value pairs for the MIT Data Warehouse connection.
SYMPLECTIC_FTP_JSON='{"SYMPLECTIC_FTP_HOST": "<VALID_ELEMENTS_FTP_HOST>", "SYMPLECTIC_FTP_PORT": "<VALID_ELEMENTS_FTP_PORT>", "SYMPLECTIC_FTP_USER": "<VALID_ELEMENTS_FTP_USER>", "SYMPLECTIC_FTP_PASS": "<VALID_ELEMENTS_FTP_PASSWORD>"}' # A JSON formatted string of key/value pairs for connecting to the Symplectic Elements FTP server.
SYMPLECTIC_FTP_PATH="<FTP_FILE_DIRECTORY>/<FEED_TYPE>.xml" # Full XML file path that is uploaded to the Symplectic Elements FTP server.
SNS_TOPIC="<VALID_SNS_TOPIC_ARN>" # SNS topic ARN used for sending email notifications.
```

## Optional ENV

* `LOG_LEVEL` = The log level for the `carbon` application. Defaults to `INFO` if not set.
* `ORACLE_LIB_DIR` = The directory containing the Oracle Instant Client library.
* `SENTRY_DSN` = If set to a valid Sentry DSN, enables Sentry exception monitoring. This is not needed for local development.
### Optional
```shell
LOG_LEVEL="INFO" # The log level for the 'carbon' application. Defaults to 'INFO' if not set.
ORACLE_LIB_DIR="<PATH>" # The directory containing the Oracle Instant Client library.
SENTRY_DSN="<SENTRY_DSN>" # If set to a valid Sentry DSN, enables Sentry exception monitoring. This is not needed for local development.
```
4 changes: 3 additions & 1 deletion carbon/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class CarbonFtpsTls(FTP_TLS):
See ftplib.FTP_TLS for more details.
"""

def ntransfercmd(self, cmd: str, rest: str | int | None = None) -> tuple[socket, int]:
def ntransfercmd(
self, cmd: str, rest: str | int | None = None
) -> tuple[socket, int | None]:
Comment on lines +41 to +43
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused by this at first but it looks like the type hint was indeed updated, see #4 here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! Helpful to understand why it changed.

"""Initiate a transfer over the data connection."""
conn, size = FTP.ntransfercmd(self, cmd, rest)
if self._prot_p: # type: ignore[attr-defined]
Expand Down
10 changes: 5 additions & 5 deletions carbon/feed.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def records(self) -> Generator[dict[str, Any], Any, None]:
yield dict(zip(result.keys(), row, strict=True))

@abstractmethod
def _add_element(self, record: dict[str, Any]) -> None | ET._Element: # noqa: SLF001
def _add_element(self, record: dict[str, Any]) -> None | ET._Element:
"""Create an XML element for a provided record.

Must be overridden by subclasses.
Expand All @@ -70,11 +70,11 @@ def _add_element(self, record: dict[str, Any]) -> None | ET._Element: # noqa: S

def _add_subelement(
self,
parent: ET._Element, # noqa: SLF001
parent: ET._Element,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but always appreciate when we can remove linting overrides / ignores.

element_name: str,
element_text: str | None = None,
**kwargs: str,
) -> ET._Element: # noqa: SLF001):
) -> ET._Element:
"""Add an XML subelement to an existing element.

Args:
Expand Down Expand Up @@ -115,7 +115,7 @@ class ArticlesXmlFeed(BaseXmlFeed):
.where(aa_articles.c.MIT_ID.is_not(None))
)

def _add_element(self, record: dict[str, Any]) -> ET._Element: # noqa: SLF001
def _add_element(self, record: dict[str, Any]) -> ET._Element:
"""Create an XML element representing an article.

The function will create a single 'ARTICLE' element that contains subelements
Expand Down Expand Up @@ -277,7 +277,7 @@ class PeopleXmlFeed(BaseXmlFeed):
.where(func.upper(persons.c.JOB_TITLE).in_(titles))
)

def _add_element(self, record: dict[str, Any]) -> ET._Element: # noqa: SLF001
def _add_element(self, record: dict[str, Any]) -> ET._Element:
"""Create an XML element representing a person.

The function will create a single 'record' element that contains subelements
Expand Down
45 changes: 23 additions & 22 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,57 +10,58 @@ exclude = ["tests/"]
log_level = "INFO"

[tool.ruff]
target-version = "py311"
target-version = "py312"

# set max line length
line-length = 90

# enumerate all fixed violations
show-fixes = true

[tool.ruff.lint]
select = ["ALL", "PT"]

ignore = [
# default
"ANN101",
"ANN102",
"COM812",
"D107",
"N812",
"PTH",
"ANN101",
"ANN102",
"COM812",
"D107",
"N812",
"PTH",

# project-specific
"C90",
"D100",
"D101",
"D101",
"D102",
"D103",
"D104",
"D104",
"PLR0912",
"PLR0913",
"PLR0913",
"PLR0915",
"S320",
"S321",
"S321",
]

# allow autofix behavior for specified rules
fixable = ["E", "F", "I", "Q"]

# set max line length
line-length = 90

# enumerate all fixed violations
show-fixes = true

[tool.ruff.flake8-annotations]
[tool.ruff.lint.flake8-annotations]
mypy-init-return = true

[tool.ruff.flake8-pytest-style]
[tool.ruff.lint.flake8-pytest-style]
fixture-parentheses = false

[tool.ruff.per-file-ignores]
[tool.ruff.lint.per-file-ignores]
"tests/**/*" = [
"ANN",
"ARG001",
"S101",
]

[tool.ruff.pycodestyle]
[tool.ruff.lint.pycodestyle]
max-doc-length = 90

[tool.ruff.pydocstyle]
[tool.ruff.lint.pydocstyle]
convention = "google"
5 changes: 3 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Carbon: a people loader."""

import subprocess

from setuptools import find_packages, setup
Expand All @@ -7,8 +8,8 @@
mit_license = f.read()

try:
output = subprocess.run(
["git", "describe", "--always"], # noqa: S603, S607
output = subprocess.run( # noqa: S603
["git", "describe", "--always"], # noqa: S607
stdout=subprocess.PIPE,
encoding="utf-8",
check=False,
Expand Down