request body in openapi no need to append the components #2069
Conversation
|
Not quite sure so I want to ask, does this PR fix this issue: #2029 ? |
|
Thanks, very glad to hear it. Please extend your commit message to reference that issue with a "Fixes" comment like shown above. |
frankzye
force-pushed
the
bugfix/request-body-no-components
branch
from
806757c to
c3f4917
Compare
frankzye
force-pushed
the
bugfix/request-body-no-components
branch
from
c3f4917 to
b74da36
Compare
updated |
|
This project has an enormous suite of automated tests. Would you please consider adding a new test that checks the revised behavior? I.e., the test should fail on released version 3.2, succeed with this change. |
Signed-off-by: frankzye1 <[email protected]>
for more information, see https://pre-commit.ci
added with tests |
|
@Ruwann @RobbeSneyders would one of you maintainers please approve a run of the GitGHub workflow to validate this change? |
| def test_openapi_schema_validate_with_request_body_change(simple_app): | ||
| app_client = simple_app.test_client() | ||
|
|
||
| if simple_app._spec_file == "openapi.yaml": |
There was a problem hiding this comment.
Would you please help me understand the need for this if clause? Are there multiple instances of simple_app and some have a different spec file? I suppose you are trying to increase the chance that the app is listening at endpoint /v1.0/test-default-object-body. Seems like all the other tests in this file have this kind of assumption, so I'm not sure it's necessary to check in this one. Or maybe I'm missing something huge :)
There was a problem hiding this comment.
One more comment, if you can please have patience with me. I see there is a test file test_responses.py. Because this new test case doesn't actually exercise any parameters, maybe you might consider moving this new case to that file?
| def with_definitions(self, schema: dict): | ||
| if self.components: | ||
| def with_definitions(self, schema: dict, append_components=True): | ||
| if self.components and append_components: |
There was a problem hiding this comment.
I realize that only the init method has pydoc for parameters, the rest of the methods have basically none. With that said, since you are introducing a new parameter with a default value, I think it would be really helpful to add a little pydoc section with a sentence about the new parameter. Please consider it. I'd like to suggest that text, but I am still trying to understand exactly what is being appended here (or not).
Fixes # .
reproduce:
error:
Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^\/']['patternProperties']['^(get|put|post|delete|options|head|patch|trace)$']['properties']['requestBody']:
{'oneOf': [{'$ref': '#/definitions/RequestBody'},
{'$ref': '#/definitions/Reference'}]}
Changes proposed in this pull request:
request body in openapi no need to append the components