Skip to content

USE 559 - bulk update fulltexts#388

Merged
ghukill merged 2 commits into
mainfrom
USE-559-bulk-update-fulltexts
May 21, 2026
Merged

USE 559 - bulk update fulltexts#388
ghukill merged 2 commits into
mainfrom
USE-559-bulk-update-fulltexts

Conversation

@ghukill

@ghukill ghukill commented May 20, 2026

Copy link
Copy Markdown
Contributor

NOTE: this PR builds on #387. Will change base on merge of that PR.

Purpose and background context

This PR adds a new CLI command bulk-update-fulltexts to TIM. This is a near clone of pre-existing CLI command bulk-update-embeddings.

As noted in the ticket and commit, it was decided to accept some code duplication in the name of simplicity at the moment. There may be a way to combine these, something like bulk-update with a TIMDEX "data type" flag to set, but it would require some helper methods and branching as the logic does differ slightly. Given there is some StepFunction refactoring anticipated in coming months, felt more aligned with that work.

With this CLI command added, we'll have the ability to add a new block in the TIMDEX StepFunction to update documents with harvested fulltext (first use case, DSpace theses).

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

The following will run the bulk-update-fulltexts command for a small run of researchdatabases in Dev. Note that the fulltext records are just simulated records for this source, but do exercise the CLI command correctly.

1- Set Dev1 TimdexManager credentials

2- Set env vars:

WORKSPACE=dev
WARNING_ONLY_LOGGERS=boto3,botocore,opensearch,urllib3
TDA_LOG_LEVEL=DEBUG
AUTH_SERVICE_TYPE=es
TIMDEX_OPENSEARCH_ENDPOINT=search-timdex-dev-fgby3dckzlfmni2len2wbdf444.us-east-1.es.amazonaws.com
TIMDEX_DATASET_LOCATION=s3://timdex-extract-dev-222053980223/dataset

3- Run CLI command:

uv run --env-file .env tim --verbose \
bulk-update-fulltexts \
-s researchdatabases \
--run-id=18cbec87-ce25-4355-a526-caaa3eb1ac1e \
s3://timdex-extract-dev-222053980223/dataset

Should see output:

Bulk update with fulltexts complete: {"updated": 901, "skipped": 0, "errors": 0, "total": 901}

Includes new or updated dependencies?

NO

Changes expectations for external applications?

YES: unlocks a StepFunction update to add a new fulltext harvesting + indexing section

What are the relevant tickets?

Code review

  • Code review best practices are documented here and you are encouraged to have a constructive dialogue with your reviewers about their preferences and expectations.

ghukill added 2 commits May 20, 2026 16:02
Why these changes are being introduced:

Nearly identical to embeddings, the TIMDEX dataset now contains "fulltexts" which are
added to Opensearch documents via bulk updating.

How this addresses that need:

A new CLI command `bulk-update-fulltexts` is added that is a near 1:1 clone of
`bulk-update-embeddings`.  It felt premature to look for a way to combine those
CLI commands just to DRY things up, given there are some slight differences
and the StepFunction may soon get some refactoring which could have bearing here.

Instead, a simple, boring clone of the `bulk-update-embeddings` was done, adjusting
for fulltexts, and it's working as expected.

Side effects of this change:
* Ability to update Opensearch documents with fulltexts

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/USE-559
@ghukill ghukill changed the base branch from main to TIMX-634-reduce-payload-size-default May 20, 2026 20:05
@ghukill ghukill changed the base branch from TIMX-634-reduce-payload-size-default to main May 20, 2026 20:12
@ghukill ghukill marked this pull request as ready for review May 20, 2026 20:34
@ghukill ghukill requested a review from a team as a code owner May 20, 2026 20:34
@jonavellecuerdo jonavellecuerdo self-assigned this May 21, 2026
@jonavellecuerdo

Copy link
Copy Markdown
Contributor

Ran the code shared above and received the following output:

2026-05-21 09:20:52,113 INFO tim.cli.bulk_update_fulltexts() line 502: Bulk update with fulltexts complete: {"updated": 0, "skipped": 901, "errors": 0, "total": 901}

Makes sense! Given you ran it previously. Looks good to me!

@ghukill ghukill merged commit 28da553 into main May 21, 2026
5 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.

2 participants