Skip to content

USE 449 - Bump TDA to version 4.1#382

Merged
ghukill merged 2 commits into
mainfrom
USE-449-update-tda-4.1
Mar 16, 2026
Merged

USE 449 - Bump TDA to version 4.1#382
ghukill merged 2 commits into
mainfrom
USE-449-update-tda-4.1

Conversation

@ghukill

@ghukill ghukill commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Purpose and background context

This PR bumps timdex-dataset-api (TDA) to version 4.1, which supports duckdb==1.5.

This PR also fixess a latent bug.

Since the migration from pipenv to uv, we have not performed a "full" run in an AWS context which utilizes the Docker container. It turns out that I forgot to include the config/ directory in the Docker image, which is an issue when we use the opensearch.create_index() helper function.

This commit copies the config/ directory into the Docker image and rectifies that. Exploring more robust solutions as follow-up work, e.g. full paths, for the file to ensure that local testing would have exposed that.

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

See this successful run of StepFunction in Dev1 to demonstrate:

  1. TDA 4.1 working as expected
  2. Bug fixed for Docker container and config/ directory

Includes new or updated dependencies?

YES

Changes expectations for external applications?

NO

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 March 16, 2026 09:57
Why these changes are being introduced:

There was a latent bug that was not exposed until a "full" run was performed in Dev1.
We have not used the `opensearch.create_index()` function in AWS since the migration from
pipenv to uv, which is failing.  This is because the `config/` directory was not included
in the Docker image, and thus that import fails.

Going to look into a more robust solution, though this works for now.

How this addresses that need:

Copies `config/` directory into Docker image.

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/USE-449
@ghukill ghukill marked this pull request as ready for review March 16, 2026 14:47
@ghukill ghukill requested a review from a team as a code owner March 16, 2026 14:47
Comment thread Dockerfile
# Copy source
# Copy source and config
COPY tim ./tim
COPY config ./config

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.

Good catch!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! It was a sneaky one. Couldn't figure out why things were "fine" in production.... until realizing we just hadn't hit a code path that required this file yet 😅.

@ghukill ghukill merged commit 0f34d7a into main Mar 16, 2026
3 checks passed
@ghukill ghukill deleted the USE-449-update-tda-4.1 branch April 7, 2026 15:14
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