Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set 'default' field type as anything possible #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MathieuAvila
Copy link

Default can be anything.

Same problem than MR #104 : unable to write UT, the following error occurs:

============================================================================================== ERRORS =============================================================================================== ______________________________________________________________________________ ERROR collecting tests/fastapi_test.py _______________________________________________________________________________ tests/fastapi_test.py:15: in <module> from api.main import app tests/api/main.py:59: in <module> def getPet(pet_id: int = Query(..., alias='petId')) -> Pets: ../.local/lib/python3.10/site-packages/fastapi/routing.py:706: in decorator self.add_api_route( ../.local/lib/python3.10/site-packages/fastapi/routing.py:645: in add_api_route route = route_class( ../.local/lib/python3.10/site-packages/fastapi/routing.py:491: in __init__ self.dependant = get_dependant(path=self.path_format, call=self.endpoint) ../.local/lib/python3.10/site-packages/fastapi/dependencies/utils.py:261: in get_dependant type_annotation, depends, param_field = analyze_param( ../.local/lib/python3.10/site-packages/fastapi/dependencies/utils.py:410: in analyze_param assert isinstance(field_info, params.Path), ( E AssertionError: Cannot use Queryfor path param 'pet_id' ====================================================================================== short test summary info ====================================================================================== ERROR tests/fastapi_test.py - AssertionError: Cannot useQuery for path param 'pet_id' !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! ========================================================================================= 1 error in 0.49s ==========================================================================================

@Dorthu
Copy link
Owner

Dorthu commented Aug 3, 2023

The jsonschema spec says the following about default:

This keyword can be used to supply a default JSON value associated
with a particular schema. It is RECOMMENDED that a default value be
valid against the associated schema.

That's not exactly prescriptive, but per RFC-2119:

  1. SHOULD This word, or the adjective "RECOMMENDED", mean that there
    may exist valid reasons in particular circumstances to ignore a
    particular item, but the full implications must be understood and
    carefully weighed before choosing a different course.

As written today, this example schema is invalid:

Example:
  type: integer
  default: "this is a string"

That's clearly an oversimplified example, but I guess the question is: why would default need to accept a value that isn't otherwise valid against the schema it's present in?

@@ -88,7 +88,7 @@ def _parse_data(self):
self.additionalProperties = self._get("additionalProperties", [bool, dict])
self.description = self._get("description", str)
self.format = self._get("format", str)
self.default = self._get("default", TYPE_LOOKUP.get(self.type, str)) # TODO - str as a default?
self.default = self._get("default", [list, dict, str, int, float, bool]) # TODO - str as a default?
Copy link
Owner

@Dorthu Dorthu Aug 3, 2023

Choose a reason for hiding this comment

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

If we wanted to accept any type here, a wildcard would be more appropriate:

Suggested change
self.default = self._get("default", [list, dict, str, int, float, bool]) # TODO - str as a default?
self.default = self._get("default", "*")

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.

2 participants