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

Manage dict used as 'additionalProperties' #104

Closed
wants to merge 1 commit into from

Conversation

MathieuAvila
Copy link

Sorry, I was unable to test nor write unit test. I get the following error, tell me how to fix and I'll write the associated UT.

============================================================================================== 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 ==========================================================================================

@MathieuAvila
Copy link
Author

Humn this seems to be a duplicate of #63 , except that my understanding is that additionalProperties is used to create a dictionary.

@Dorthu
Copy link
Owner

Dorthu commented Aug 3, 2023

Reading back over #63, I think this needs to accept a bool for additionalProperties, and to respect its value. Namely, if additionalProperties is True, any additional properties should be accepted, and if it is False than none must be allowed. If not specified, True is the default, and if given as a schema, only the named properties are accepted, and they're all optional.

The previous PR was very close to the intended behavior, and it might be better to start there and build out the features it was missing.

@MathieuAvila
Copy link
Author

Ok, closing this and will look at #63. My goal was to be compatible with fastapi's dict serialization, but it seems we have to interpret the specs a little.

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