Skip to content

Conversation

ehanson8
Copy link
Contributor

@ehanson8 ehanson8 commented May 7, 2025

Purpose and background context

Add command to validate all AIPs in the current AWS environment.

Includes recent pip-audit updates.

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

Set credentials for Dev, Stage, or Prod and run the following command:

pipenv run cli validate-all --output-csv-filepath output/validate_all.csv

Observe app's logging statements and verify that the validate_all.csv is created and populated.

Includes new or updated dependencies?

YES

Changes expectations for external applications?

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

ehanson8 added 2 commits May 7, 2025 09:49
Why these changes are being introduced:
* Add CLI command to validate all AIPs in a given AWS environment

How this addresses that need:
* Add validate-all CLI command and corresponding CLI test
* Add logging statement to bulk-validate CLI command and update corresponding CLI tests

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/IR-233
@ehanson8 ehanson8 requested a review from a team as a code owner May 7, 2025 15:54
Copy link
Contributor

@ghukill ghukill left a comment

Choose a reason for hiding this comment

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

All in all, looks great to me!

Requesting two changes:

  1. carry over all bulk-validate arguments
  2. remove a file object .seek() so as not to suggest to future code readers that the temp_file file object is in play in that way

* Add bulk-validate args to validate-all
* Remove unnecessary .seek() call
@ehanson8 ehanson8 requested a review from ghukill May 7, 2025 19:33
@coveralls
Copy link

Pull Request Test Coverage Report for Build 14891726606

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 88.499%

Totals Coverage Status
Change from base Build 14798719851: 0.4%
Covered Lines: 631
Relevant Lines: 713

💛 - Coveralls

Copy link
Contributor

@ghukill ghukill left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work.

@ehanson8 ehanson8 merged commit 082700a into main May 8, 2025
3 checks passed
@ehanson8 ehanson8 deleted the IR-233-validate-all branch May 8, 2025 13:19
ehanson8 added a commit that referenced this pull request May 9, 2025
* Add pip-audit and update dependencies

* IR-233 validate all

Why these changes are being introduced:
* Add CLI command to validate all AIPs in a given AWS environment

How this addresses that need:
* Add validate-all CLI command and corresponding CLI test
* Add logging statement to bulk-validate CLI command and update corresponding CLI tests

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/IR-233

* Updates based on discussion in PR # 96

* Add bulk-validate args to validate-all
* Remove unnecessary .seek() call
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.

3 participants