Skip to content
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

Tests and Python Upgrade #4

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Tests and Python Upgrade #4

merged 3 commits into from
Jan 17, 2024

Conversation

cabutlermit
Copy link
Contributor

@cabutlermit cabutlermit commented Jan 17, 2024

Purpose and background context

A few minor changes: this adds additional tests to verify that the code processes the origin-request JSON objects correctly, it updates the Python version to 3.11 to match what is now available in AWS with the update AWS Provider in Terraform, and it removes any Sentry references since we don't use Sentry in Lambda@Edge functions (for now?).

Additionally, this PR (and its eventual merge to main will be a test of the automated deployment of the Lambda function in Stage-Workloads. This code is already running in Stage-Workloads as part of the infrastructure updates and verifying that the make upload-zip command can write a file to the Stage-Workloads S3 shared-files-xxxx bucket. Once this PR is approved, we'll track the automation behavior when it is merged to main.

How can a reviewer manually see the effects of these changes?

  • Checkout this branch locally and run make tests to see all the tests run

Includes new or updated dependencies?

YES: Updates Python version to 3.11 and removes Sentry dependency.

Changes expectations for external applications?

NO

What are the relevant tickets?

Developer

  • The Dev CF Lambda@Edge Full Deploy workflow has been run to update CloudFront in Dev1
  • All related Jira tickets are linked in commit message(s)
  • Stakeholder approval has been confirmed (or is not needed)

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 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

Why these changes are being introduced:
Update the testing functions to address the three different input json
files (standard cdn, future-of-libraries custom domain, grandchallenges
custom domain). Fix a typo in the Makefile

How this addresses that need:
* Clean up the json fixtures to match what's happening in the CDN
* Add testing functions for the standard CDN and the Grand Challenges
site
* Clean up the testing function for Future of Libraries site

Side effects of this change:
None.
Why these changes are being introduced:
With the updates to mitlib-tf-workloads-libraries-website, Terraform
and the AWS Provider now support Python 3.11 for Lambda and Lambda@Edge.

How this addresses that need:
* Update the Python version to 3.11

Side effects of this change:
None.

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/GDT-106
@cabutlermit cabutlermit marked this pull request as ready for review January 17, 2024 15:54
@cabutlermit
Copy link
Contributor Author

@JPrevost I'm tagging you for a review so that you can follow along with the automated deployment behavior when it merges. There isn't much to actually approve here. 😁

JPrevost
JPrevost previously approved these changes Jan 17, 2024
Copy link
Member

@JPrevost JPrevost left a comment

Choose a reason for hiding this comment

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

I suspect this is an oddity in the shared workflow this uses, but tests run twice because it runs make test which runs the tests as well as make coveralls which runs make test before it runs. It may be a minor thing as these tests are fast, but repos that have extensive test suites running twice would be annoying. The shared workflow this inherits from should likely consider just running make coveralls to avoid double test runs in CI.

@cabutlermit
Copy link
Contributor Author

I suspect this is an oddity in the shared workflow this uses, but tests run twice because it runs make test which runs the tests as well as make coveralls which runs make test before it runs. It may be a minor thing as these tests are fast, but repos that have extensive test suites running twice would be annoying. The shared workflow this inherits from should likely consider just running make coveralls to avoid double test runs in CI.

ah, crud. just realized that I had forgotten to uncomment the automation piece of stage workflow before submitting this PR! I'm going to push one more commit...

Why these changes are being introduced:
This turns on the automated trigger (on.push) for merge to `main`.

How this addresses that need:
* Uncomment the `on.push` workflow trigger for the deploy to Stage

Side effects of this change:
Merge to `main` will now automatically trigger deploys to
Stage-Workloads
Copy link

Pull Request Test Coverage Report for Build 7558825715

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+18.2%) to 100.0%

Totals Coverage Status
Change from base Build 7492753605: 18.2%
Covered Lines: 22
Relevant Lines: 22

💛 - Coveralls

@cabutlermit cabutlermit merged commit 00ee988 into main Jan 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants