Remove import from function

[lacoak21]

Change Summary

closes #1326

Overview

The circular dependency was an issue with spice_utilities.py and spice_api_metakernel.py. To fix this, I moved all of the helper functions into spice_utilities.py

[Copilot]

Pull request overview

This PR updates spice_utilities.furnish_best_spice_file() to import spice_metakernel_api at module scope instead of importing it inside the function, aiming to remove a previously-added workaround for a circular import.

Changes:

• Add a module-level import of spice_metakernel_api in spice_utilities.py.
• Remove the in-function import of spice_metakernel_api inside furnish_best_spice_file().

Suppressed comments (1)

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:90

• With the module-level import removed to avoid the circular dependency, this function still needs a local import of spice_metakernel_api so spice_metakernel_api.lambda_handler(...) is defined. Keeping this import inside the function also prevents the circular import at module load time.

    # Check if S3_BUCKET and DATA_DIR are set
    if "S3_BUCKET" not in os.environ or "DATA_DIR" not in imap_data_access.config:

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (5)

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:39

• Typo in docstring: "Planetary Contants" -> "Planetary Constants".

    Raises

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:264

• convert_input_times_to_j2000 is part of the API surface (imported by spice_metakernel_api) but it currently has no type hints and its docstring doesn’t describe accepted input formats. Adding annotations here will make the contract clearer and help catch misuse.
  sds_data_manager/lambda_code/SDSCode/spice_utilities.py:191
• KernelCollection() is instantiated repeatedly (allowed_spice_types=KernelCollection().category_types and again in the loop). Creating it once improves readability and avoids rebuilding the same lists multiple times.
  sds_data_manager/lambda_code/SDSCode/spice_utilities.py:147
• These kernel Enum definitions / KernelCollection / metakernel-building logic are now duplicated in both spice_utilities.py and api_lambdas/spice_metakernel_api.py (same class names and behavior). Keeping two copies risks drift and makes future updates error-prone; consider consolidating into a single shared implementation and importing it from the other module.

    )
    if metakernel_response["statusCode"] != 200:
        raise FileNotFoundError(
            f"Unable to find the latest {kernel_type} kernel. "
            "Please ensure that the kernel is available in the database."

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:19

• Importing api_lambdas.spice_query_api at module import time pulls in SQLAlchemy/DB dependencies whenever any caller imports spice_utilities (including pipelines that may only use download_from_s3). This can increase Lambda cold-start time and broadens the import graph; consider deferring that import to the narrowest scope (e.g., inside build_metakernel) or moving metakernel-building code into a module that already depends on the DB layer.

logger = logging.getLogger(__name__)

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (4)

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:39

• Docstring typo: "Planetary Contants" should be "Planetary Constants".

class PlanetaryConstantsKernels(Enum):
    """Container for Planetary Contants Kernel Types."""

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:253

• _metakernel_builder expects a collection of full type names (as used by spice_metakernel_api, which passes a set). Passing a plain string relies on substring semantics and is inconsistent with the helper’s contract; wrap it in a set to avoid accidental mismatches.

    metakernel = _metakernel_builder(0, MAXIMUM_MISSION_J2000_TIME, kernel_type.upper())

sds_data_manager/lambda_code/SDSCode/spice_utilities.py:288

• KernelCollection() is instantiated multiple times in _metakernel_builder, which is unnecessary and makes it easier for these values to drift if the class later gains state. Create one KernelCollection instance and reuse it for allowed_spice_types and the load-order loop.

    metakernel = MetaKernel(
        start_time,
        end_time,
        allowed_spice_types=KernelCollection().category_types,
    )

sds_data_manager/lambda_code/SDSCode/api_lambdas/spice_metakernel_api.py:10

• spice_metakernel_api is importing and using _metakernel_builder (leading underscore), but this helper is now used across module boundaries and is part of the API surface of spice_utilities. Consider renaming it to a public function (e.g., build_metakernel/metakernel_builder) or re-homing it so callers aren’t depending on a private symbol.

from ..spice_utilities import _metakernel_builder, furnish_best_spice_file

[tech3371]

Thank you for doing this. This was needed to be done for a while!!