Skip to content

[WIP] Attempts to enforce schema in dkist client response#557

Draft
Cadair wants to merge 1 commit intoDKISTDC:mainfrom
Cadair:openapi_results
Draft

[WIP] Attempts to enforce schema in dkist client response#557
Cadair wants to merge 1 commit intoDKISTDC:mainfrom
Cadair:openapi_results

Conversation

@Cadair
Copy link
Member

@Cadair Cadair commented Apr 24, 2025

No description provided.

@Cadair Cadair marked this pull request as draft April 24, 2025 10:53
@Cadair Cadair requested a review from Copilot April 24, 2025 10:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces a new module for parsing the OpenAPI specification of the dataset search API and enforcing type hints based on JSON schema definitions. Key changes include:

  • Adding a new utility function (get_spec) to load the OpenAPI spec.
  • Implementing a JsonSchemaTypeConverter class with methods to resolve JSON schema types into Python type hints.
  • Providing a helper function (search_results_type_hints) to extract type hints for search result schemas.
Comments suppressed due to low confidence (1)

dkist/utils/openapi.py:36

  • [nitpick] The error message 'No way friend' is informal and may lack clarity. Consider using a more descriptive and professional message to indicate that schema types starting with '_' are unsupported.
raise AttributeError("No way friend")

Copy link
Contributor

@SolarDrew SolarDrew left a comment

Choose a reason for hiding this comment

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

To the extent that I understand anything that's going on here, LGTM 👍

def get_type_handler(self, schema_type: str) -> Callable:
"""Get a handler from this schema draft version."""
if schema_type.startswith("_"):
raise AttributeError("No way friend")
Copy link
Contributor

Choose a reason for hiding this comment

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

A more descriptive error message would probably be useful here.

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.

3 participants