Support for type=spin/repoint/thruster/kernels for the spice-query api by leowerneck · Pull Request #1552 · IMAP-Science-Operations-Center/sds-data-manager

leowerneck · 2026-08-05T20:35:14Z

This is a slight rewrite of #1250 by @vineetbansal, which splits some of the new tests for better granularity and introduces a helper function to ensure consistent formatting of the response output.

Change Summary

Fixes issue #911

Added support to query non-SPICE tables (spin/repoint/thruster) using the /spice-query endpoint.

The type parameter decides which table to query - kernels searches through the spice table. Other possible values are spin, thruster, or repoint, which delegate to the existing lambda in non_spice_table_api.py.

When the type parameter is not set, it is assumed to be kernels, preserving legacy behavior. A type parameter set to a specific kernel type like spacecraft_clock still works as before.

Parameters like file_name, start_time and end_time that the SPICE api already supports are now mapped to the non-SPICE API, so the caller has to go through minimal changes to start using these.

File changes

Main functionality added to spice_query_api.py that internally calls non_spice_tables_api.
I had to move an import in spice_metakernel_api.py to where it is used to get around a circular import issue (spice_utilities.py imports spice_metakernel_api and spice_metakernel_api.py imports spice_utilities).

Testing

Added a test to test_spice_query_api.py to ensure that type parameter is interpreted correctly for SPICE tables.
Added tests to test_spin_api.py to ensure that calls made through spice_query.api return expected responses. Existing tests left intact, so the old API endpoints work as well.

Copilot

Pull request overview

Adds non-SPICE table routing to /spice-query while preserving kernel-query behavior.

Changes:

Routes spin, repoint, and thruster queries to the non-SPICE API.
Maps SPICE query parameters to non-SPICE equivalents.
Adds response formatting and endpoint tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
`spice_query_api.py`	Adds type routing and parameter conversion.
`non_spice_table_api.py`	Extracts query and response helpers.
`test_spin_api.py`	Tests delegated non-SPICE queries.
`test_spice_query_api.py`	Tests kernel type behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        try:
+            spiceypy.et2datetime(0)
+        except spiceypy.utils.exceptions.SpiceMISSINGTIMEINFO:
+            furnish_best_spice_file("leapseconds")


+            table.ingestion_date,
+            row_number,
+        )
+        query = select(subquery).where(subquery.c.row_num == 1)


+        row_number = (
+            func.row_number()
+            .over(
+                partition_by=(table.start_date, table.end_date),


tech3371

I had few suggestion. Sorry for taking a while. It's been a while I have looked at these components and had to regroup. I made few suggestions. If you like to chat or ask for clarification, I am happy to support.

tech3371 · 2026-08-13T20:03:19Z

looks like this code I have wrote a while back is hard to read and follow. Sorry for this and appreciate your understanding.

I am wondering if we should start fresh with logic something like this and reuse other code we have written since then or in parallel.

If table == 'spin': copy code from this https://github.com/IMAP-Science-Operations-Center/sds-data-manager/blob/dev/sds_data_manager/orchestration/spin.py::get_spin_files() elif table == 'repoint': copy code from this https://github.com/IMAP-Science-Operations-Center/sds-data-manager/blob/dev/sds_data_manager/orchestration/repoint_file.py::get_latest_repoint_file() elif table == 'small-forces': create new function that queries latest small forces file. else: return error json response of unrecognize table input.

As you work on this, I appreciate if you can think about first task in this ticket, IMAP-Science-Operations-Center/imap-data-access#346. This backend work you are doing here will support interface for that work.

Eg. that ticket will need to add CLI handler for command like this. These are examples. These are not implemented yet.

imap-data-access query --table spice --type repoint --version latest imap-data-access query --table spice --type spin --start-date <> --end-date <> --version latest imap-data-access query --table spice --type thruster --start-date <> --end-date <> --version latest

tech3371 · 2026-08-13T20:18:24Z

                        models.SPICEFiles.min_date_j2000 <= float(value)
                    )
-                elif param == "type":
+                elif param == "type" and value != "kernels":


popping 'type' and generalizing to 'kernel' is making me concern because type may be used to filter kernels elsewhere and many user interfaces such as website support query by specific kernel type and return result of that kernel type. I have a feeling that they use this filter logic to return requested kernel. Eg. type=attitude_history will filter and return just attitude history kernel.

In general, I would be careful changing logic in here file since it's heavily used by all pipeline and user interface. This is where I appreciate double check form @bryan-harter .

ok..I looked again more carefully and looks like you get queryStringParameters again at line 90 and there you are not popping type. good.

tech3371 · 2026-08-13T20:30:01Z

+    * ``<other>`` - A specific SPICE kernel type from the SPICE files table
+
+    Passing a kernel-type value such as ``attitude_history`` filters the SPICE table
+    by ``kernel_type``. The remaining SPICE-specific parameters
+    (``file_name``, ``start_time``, ``end_time``, ``latest``,
+    ``start_ingest_date``, ``end_ingest_date``) apply.
+
+    When ``type`` is one of the non-SPICE values, the ``file_name`` parameter is
+    renamed to ``file_path``, ``start_time`` is mapped to ``start_date``, ``end_time``
+    is mapped to ``end_date``, and the remaining parameters are passed to the
+    non-SPICE API.


I would consolidate to something like this:

Suggested change

* ``<other>`` - A specific SPICE kernel type from the SPICE files table

Passing a kernel-type value such as ``attitude_history`` filters the SPICE table

by ``kernel_type``. The remaining SPICE-specific parameters

(``file_name``, ``start_time``, ``end_time``, ``latest``,

``start_ingest_date``, ``end_ingest_date``) apply.

When ``type`` is one of the non-SPICE values, the ``file_name`` parameter is

renamed to ``file_path``, ``start_time`` is mapped to ``start_date``, ``end_time``

is mapped to ``end_date``, and the remaining parameters are passed to the

non-SPICE API.

* ``<other kernel type>`` - Use metakernel API to query and return requested kernels.

tech3371 · 2026-08-13T20:31:05Z

+    query_params = event.get("queryStringParameters", {})
+    table_type = query_params.get("type", "kernels")


Consolidating to reduce confusion with query_params at line 90.

Suggested change

query_params = event.get("queryStringParameters", {})

table_type = query_params.get("type", "kernels")

table_type = event.get("queryStringParameters", {}).get("type", "")

tech3371 · 2026-08-13T20:32:29Z

+        # Remove `type`, since it is not a valid non-SPICE query parameter.
+        query_params.pop("type")


I would avoid this line or move it into _remap_to_non_spice_params() to reduce confusion.

Suggested change

# Remove `type`, since it is not a valid non-SPICE query parameter.

query_params.pop("type")

tech3371 · 2026-08-13T20:37:45Z

+        return non_spice_table_api.get_json_response(status_code, search_results)
+
+
+def _remap_to_non_spice_params(query_params: dict) -> dict:


I don't think we need this function. We can expect user to pass human readable time, yyyymmdd. We normally convert str to datetime to be able to query DB. @tmplummer do we need to support hhmmss precision for these kinds of query?

Also, I don't think lambda layer has spiceypy library unless it has changed recently that I don't know of.

This is just for imap-data-access query --table spice right? I don't think that we need to support higher precision than day if that is the case. It is needed in the spice and metakernel APIs. I also don't see any need to support queries where the times are ET times instead of yyyymmdd strings.

tech3371 · 2026-08-13T20:40:30Z

+        # Remap incoming parameters to ones supported by the non-SPICE tables API.
+        try:
+            query_params = _remap_to_non_spice_params(query_params)
+        except ValueError:
+            err_msg = "Expected start/end times in ET."
+            return non_spice_table_api.get_json_response(status_code, err_msg)


See my comment on the _remap_to_non_spice_params()

tmplummer · 2026-08-14T16:15:26Z

Sorry for commenting before I had the full context of this PR and the issues driving it. It seems to me there are some inconsistencies in our current approach to handling spin, repoint, and sff files. They are being handled separately from spice files and ingested into their own tables when I think that they can in fact be handled almost exactly like the other spice kernels. When ingesting those file types, we should be able to extract all the same details that are extracted when any other spice kernel is ingested, namely:

file_name
file_root
kernel_type
version
min_date_j2000
max_date_j2000
file_intervals_j2000
min_date_datetime
max_date_datetime
file_intervals_datetime
min_date_sclk
max_date_sclk
file_intervals_sclk
sclk_kernel
lsk_kernel
ingestion_date
timestamp

If we migrated the DB to include these files in the spice table, then the spice-api as is would support querying these file types and the metakernel api would also correctly prioritize the correct file for a given time. Then, there would just remain the issue of supporting querying the spice table via the query API.

@tech3371, @bryan-harter what do you think about that approach?

tech3371 · 2026-08-17T20:36:52Z

Sorry for commenting before I had the full context of this PR and the issues driving it. It seems to me there are some inconsistencies in our current approach to handling spin, repoint, and sff files. They are being handled separately from spice files and ingested into their own tables when I think that they can in fact be handled almost exactly like the other spice kernels. When ingesting those file types, we should be able to extract all the same details that are extracted when any other spice kernel is ingested, namely:

file_name

file_root

kernel_type

version

min_date_j2000

max_date_j2000

file_intervals_j2000

min_date_datetime

max_date_datetime

file_intervals_datetime

min_date_sclk

max_date_sclk

file_intervals_sclk

sclk_kernel

lsk_kernel

ingestion_date

timestamp

If we migrated the DB to include these files in the spice table, then the spice-api as is would support querying these file types and the metakernel api would also correctly prioritize the correct file for a given time. Then, there would just remain the issue of supporting querying the spice table via the query API.

@tech3371, @bryan-harter what do you think about that approach?

I think the reason we had to set up different tables were because those files don't have as much details as SPICE kernels. Most of those other API's files are CSV or ASCII file. I think that was the reason for setting up different table. Also, I was lack SPICE knowledge and didn't want to mess up SPICE API in case those files are included in metakernel file query and user face kernel loading issues because they were expecting only Kernel types in their metakernel files. @bryan-harter Do you see Tim's suggestion as possibility? If so, that would simplify things for sure!

…ssues

…da_handler

…py: dixed missing 'small-forces' option from error message

tmplummer requested review from bryan-harter and tech3371 August 7, 2026 14:56

bryan-harter requested a balanced review from Copilot August 7, 2026 22:24

Copilot started reviewing on behalf of bryan-harter August 7, 2026 22:24 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

leowerneck assigned vineetbansal Aug 10, 2026

tech3371 reviewed Aug 13, 2026

View reviewed changes

leowerneck added 6 commits September 18, 2026 09:58

Rebased and slightly refactored code from vineetbansal:vb/issue911a

32f0c36

Added docstring to get_json_response function; addressed formatting i…

e7af5fe

…ssues

Addressed formatting issues

b3842c6

Addressed formatting issues by refactoring _get_query() logic in lamb…

3812feb

…da_handler

tests/lambda_endpoints/test_spin_api.py: fixed formatting

ebe686d

sds_data_manager/lambda_code/SDSCode/api_lambdas/non_spice_table_api.…

ee06908

…py: dixed missing 'small-forces' option from error message

leowerneck force-pushed the lw/issue911 branch from 6e15052 to ee06908 Compare September 18, 2026 13:58

		query_params = event.get("queryStringParameters", {})
		table_type = query_params.get("type", "kernels")

	query_params = event.get("queryStringParameters", {})
	table_type = query_params.get("type", "kernels")
	table_type = event.get("queryStringParameters", {}).get("type", "")

		# Remove `type`, since it is not a valid non-SPICE query parameter.
		query_params.pop("type")

		return non_spice_table_api.get_json_response(status_code, search_results)


		def _remap_to_non_spice_params(query_params: dict) -> dict:

Conversation

leowerneck commented Aug 5, 2026

Change Summary

File changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

tech3371 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmplummer commented Aug 14, 2026

Uh oh!

tech3371 commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants