Skip to content

INFRA-597-input-metadata-fix#390

Merged
ehanson8 merged 1 commit into
mainfrom
INFRA-597-input-metadata-fix
Dec 12, 2025
Merged

INFRA-597-input-metadata-fix#390
ehanson8 merged 1 commit into
mainfrom
INFRA-597-input-metadata-fix

Conversation

@ehanson8

Copy link
Copy Markdown
Contributor

Purpose and background context

Bug fix based on this Jira ticket. It also revealed there were no unit tests for validate_aip_bulk_worker so those were added as well.

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

View unit tests, I did a run on prod with a CSV from Charlie that confirmed all input fields are now present

Includes new or updated dependencies?

YES

Changes expectations for external applications?

NO

What are the relevant tickets?

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

Why these changes are being introduced:
* Input metadata was not being added to valid items as it was with invalid items

How this addresses that need:
* Update validate_aip_bulk_worker to include all input metadata for valid items
* Add TestValidateAipBulkWorker class with associated unit tests
* Update test_validate_secret_mismatch to address linting error
* Update dependencies

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/INFRA-597
@ehanson8
ehanson8 requested a review from a team as a code owner December 12, 2025 20:03

@ghukill ghukill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch and update! Looks good.

Comment thread lambdas/cli.py
Comment on lines +377 to +393
updated_row = row.to_dict()
updated_row.update(
{
"bucket": result.get("bucket"),
"aip_uuid": result.get("aip_uuid", aip_uuid),
"aip_s3_uri": result.get("aip_s3_uri", s3_uri),
"valid": bool(result.get("valid", False)),
"error": result.get("error"),
"error_details": (
json.dumps(result.get("error_details"))
if result.get("error_details") is not None
else None
),
"elapsed": result.get("elapsed"),
}
)
results_df.loc[row_index] = updated_row

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ahhhh, assuming in the previous approach it was setting columns to None or np.NaN if they weren't part of the update? Either way, if this fixes it, awesome.

@ehanson8
ehanson8 merged commit 42cab9b into main Dec 12, 2025
4 checks passed
@ehanson8
ehanson8 deleted the INFRA-597-input-metadata-fix branch December 12, 2025 21:30
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.

2 participants