Skip to content

Conversation

dwinston
Copy link
Collaborator

@dwinston dwinston commented Sep 24, 2025

On this branch, I add a fastapi.BackgroundTasks parameter to the path operation function for the GET /nmdcschema/linked_instances endpoint, and schedule a task -- to be performed after the endpoint returns -- that drops any temporary linked-instances collections that were generated earlier than one day prior to the task being run.

Related issue(s)

Fixes #1240

Related subsystem(s)

  • Runtime API (except the Minter)

Testing

  • I tested these changes (explain below)

I tested these changes by inspection. Calling the endpoint locally dropped local stale temporary collections.

Documentation

  • Other (explain below)

N/A

Maintainability

  • Every Python function I defined includes a docstring (test functions are exempt from this)
  • Every Python function parameter I introduced includes a type hint (e.g. study_id: str)
  • All "to do" or "fix me" Python comments I added begin with either # TODO or # FIXME
  • I used black to format all the Python files I created/modified
  • The PR title is in the imperative mood (e.g. "Do X") and not the declarative mood (e.g. "Does X" or "Did X")

eecavanna
eecavanna previously approved these changes Sep 24, 2025
Copy link
Collaborator

@eecavanna eecavanna left a comment

Choose a reason for hiding this comment

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

This looks good to me, and I understand how it works based on our discussion earlier "today" (thanks for introducing me to this FastAPI feature).

I'm OK with this being merged in as is.

If time permits (note: I can add this via a future PR if it is not added to this one), I'd prefer there be a logging.info(f"Dropping temporary collection: {collection_name}") statement in the cleanup function, since AFAIK it's the first use of the FastAPI BackgroundTask in the codebase and I think having a logging statement there will help us (maintainers of the deployments) internalize its behavior.

@eecavanna
Copy link
Collaborator

Edit: I approve of this being merged in once the tests pass (I just noticed some failed). No need for subsequent review by me after updating tests.

@eecavanna
Copy link
Collaborator

Thanks for implementing this "clean-up" functionality. This task remaining unresolved beyond this quarter, is not something I'm worried about. The changes made sense to me when I reviewed the branch at commit a8d1945. I'd be OK taking over trying to get the tests to pass (starting next week).

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.

Clean up temporary MongoDB collections created by /nmdcschema/linked_instances API endpoint
2 participants