-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
In 896 app stabilization #145
Conversation
369d4f6
to
266f084
Compare
Pull Request Test Coverage Report for Build 5895633125
💛 - Coveralls |
Why these changes are being introduced: * Keeping our Python version and dependencies updated is good practice and allows us to take advantage of the latest security updates and bug fixes for Python. How this addresses that need: * Update Dockerfile * Update Pipfile * Update .python-version Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-896
Why these changes are being introduced: * We updated our method for managing linters and code formatting for our Python projects to simplify configuration and use. How this addresses that need: * Apply standard pyproject.toml file * Deprecate setup.cfg * Apply linting with Ruff * Set black, mypy, ruff, and safety as linters * Add pre-commit hooks * Update Makefile * Update Pipfile Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-896
* Update imports, type hinting, and datetime calls in webhook modules * Update mypy command in Makefile * Update dependencies * Update _test_env fixture and datetime calls in conftest module * Update test_validate_missing_signature_returns_false with more explicit error check
266f084
to
1cce502
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, too! Left a comment, but fine to merge.
os.environ["AWS_ACCESS_KEY_ID"] = "testing" | ||
os.environ["AWS_DEFAULT_REGION"] = "us-east-1" | ||
os.environ["AWS_SECRET_ACCESS_KEY"] = "testing" | ||
os.environ["AWS_SECURITY_TOKEN"] = "testing" | ||
os.environ["AWS_SESSION_TOKEN"] = "testing" | ||
os.environ["ALMA_CHALLENGE_SECRET"] = "itsasecret" | ||
os.environ["ALMA_POD_EXPORT_JOB_NAME"] = "PPOD Export" | ||
os.environ["ALMA_TIMDEX_EXPORT_JOB_NAME_PREFIX"] = "TIMDEX Export" | ||
os.environ["ALMA_BURSAR_EXPORT_JOB_NAME_PREFIX"] = "Export to bursar" | ||
os.environ["LAMBDA_FUNCTION_URL"] = "http://example.com/lambda" | ||
os.environ[ | ||
"PPOD_STATE_MACHINE_ARN" | ||
] = "arn:aws:states:us-east-1:account:stateMachine:ppod-test" | ||
os.environ[ | ||
"TIMDEX_STATE_MACHINE_ARN" | ||
] = "arn:aws:states:us-east-1:account:stateMachine:timdex-test" | ||
os.environ[ | ||
"BURSAR_STATE_MACHINE_ARN" | ||
] = "arn:aws:states:us-east-1:account:stateMachine:bursar-test" | ||
os.environ["VALID_POD_EXPORT_DATE"] = "2022-05-23" | ||
os.environ["WORKSPACE"] = "test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this change was from ruff
and I think it's okay as-is, but we might want to keep our eyes open for ways in which the testing environment is set across projects.
For one, the repeating os.environ[KEY] = VALUE
could be performed in a loop from a list of key/value pairs. And that begs the question whether or not those key/value pairs should be defined in the fixture itself, or somewhere else (example you can define a pytest.ini
file, or even the pyproject.toml
)
What does this PR do?
Update the alma-webhook-lambdas repository to use new Python version and dependencies, as well as the new linting and code formatting configurations for our Python projects.
Helpful background context
The main updates introduced in this PR are to:
How can a reviewer manually see the effects of these changes?
make install
(Pipfile.lock
has been updated with fresh install of dependencies and therefore are updated)make lint
make test
make dist-dev
(The command to build the Docker image with Python 3.11 runs successfully, but the application itself still needs to be tested.)Includes new or updated dependencies?
YES
What are the relevant tickets?
https://mitlibraries.atlassian.net/browse/IN-896
Developer
Code Reviewer