Release by descriptor work - #1533
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for driving “release” operations from a per-instrument manifest file that specifies releases by (instrument, data_level/data_type, descriptor, release_flag), and refactors “latest version” selection logic to be reusable across release paths.
Changes:
- Replaces exclude-list based releasing with manifest-driven selection (including
*wildcards) for science and ancillary releases. - Introduces a shared SQLAlchemy window-function query builder to select latest-version science files.
- Updates and reshapes release API integration tests to cover the new manifest format and selection behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
tests/lambda_endpoints/test_release_api.py |
Updates release API tests to use manifest-driven release selection and new helper entry points. |
sds_data_manager/lambda_code/SDSCode/database/models.py |
Centralizes the “logical file identity” column set for version grouping/indexing. |
sds_data_manager/lambda_code/SDSCode/api_lambdas/utils.py |
Adds build_latest_version_query() for selecting latest-version science files via a window function. |
sds_data_manager/lambda_code/SDSCode/api_lambdas/release_api.py |
Implements manifest-driven releasing and refactors latest-version selection for science/ancillary files. |
Comments suppressed due to low confidence (2)
sds_data_manager/lambda_code/SDSCode/api_lambdas/release_api.py:142
latest_ancillary_releasedocstring says it returns a single-column query for use in an.in_()subquery, but the function actually loads ORM rows (.all()), updates them, and returns a list ofAncillaryFiles. This mismatch makes the function contract unclear for callers and future refactors.
Returns
-------
Query
A query of a single file_path column, for use as an `.in_()`
subquery so a large release stays a single UPDATE statement.
"""
tests/lambda_endpoints/test_release_api.py:283
- This assertion message still refers to an "Excluded" file, but this scenario is now testing "latest-version only" behavior (v001 should remain unreleased because v002 is the latest). The message should reflect that to avoid confusion.
assert (
rows["imap/ancillary/swe/imap_swe_l1b-in-flight-cal_20260420_v001.csv"] is False
), "Excluded file should not be released"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7f0cd8f to
4b96c76
Compare
tmplummer
left a comment
There was a problem hiding this comment.
I gave this a quick review and found nothing obvious. I'll approve now and come back to this this afternoon when I have more time
tmplummer
left a comment
There was a problem hiding this comment.
One minor nit. I think this looks good and it is great to hear that testing in dev went well. 🎉
ea3ee0d to
48bcb6d
Compare
|
I will merge this PR Monday and test again on Monday with list of files on this ticket #1452. |
48bcb6d to
adabc6d
Compare
Upstream IMAP-Science-Operations-Center#1533 already landed the window-function version resolution as api_lambdas/utils.build_latest_version_query, so this branch's duplicate latest_version_query.py and FILE_ID_COLUMNS definition are dropped in favor of upstream's. release_api.py takes upstream's manifest-line release wholesale; the release performance test now exercises latest_science_release directly since the old lambda params are gone. Assisted-by: Claude
* use window approach * fix _VALID_PARAMETERS setup * revert ancillary file handling * handle invalid table with error code 400 * handle invalid table with error code 400 * style: apply ruff docstring and format fixes Assisted-by: Claude * revert release_api changes; upstream #1533 supersedes them Assisted-by: Claude
Change Summary
closes #1478
Overview
Adding support to read manifest file with release information.
File content looks like this:
Each instrument will have their own manifest.
File changes
Testing