Add new 'use_spec_url_for_base_path' boolean config option #300
Conversation
|
UTs still to be updated, but I'd like some feedback first before spending time on those, just to make sure I'm not doing anything that's obviously silly, thanks. |
|
Hello @macisamuele, @sjaensch, are you able to provide any feedback on this? Thanks |
|
Gentle bump - @macisamuele , @sjaensch |
There was a problem hiding this comment.
I'm not a big fan of adding configuration parameters that modify the behaviour respect what the official specs.
Anyway checking the threads seems like an approach like this could help with supporting additional use-cases without adding tons of complexity on the configuration understanding and processing.
So overall I'm OK with the approach.
Please make sure to address the opened issue and to create a test to verify that the configured value is used and not used according to the presence of basePath in the swagger specs.
DStape
force-pushed
the
fix-basePath-used-to-build-api-url-#299
branch
from
38be785
to
43efb90
Compare
|
Updated the doc string and added new tests. |
This new option addresses issues arising from 'basePath' not being present in a given spec. According to the 2.0 spec, "If it is not included, the API is served directly under the host. The value MUST start with a leading slash (/)". 1083636 made the SwaggerClient compliant with the spec (basePath is now implicitly set to '/' if field is missing from spec). However, this broke existing code that utilizes the SwaggerClient (see Issue Yelp#299). The current (spec-compliant) behaviour remains with this new option as it defaults to False. Overriding this default with 'True' means the path element of the URL used to retrieve the spec will be used instead.
DStape
force-pushed
the
fix-basePath-used-to-build-api-url-#299
branch
from
43efb90
to
7e7b974
Compare
|
Fixed failing python3 test (due to urllib differences between python 2 and 3). |
|
Thanks @macisamuele. |
|
One last thing @macisamuele - is there an ETA on when this will be released? (Maybe v5.1.0 given that a new option has been added?) Thanks again. |
This new option addresses issues arising from 'basePath' not being
present in a given spec.
According to the 2.0 spec, "If it is not included, the API is served directly
under the host. The value MUST start with a leading slash (/).".
1083636 made the SwaggerClient compliant with the spec (basePath is now
implicitly set to '/' if field is missing from spec). However, this broke
existing code that utilizes the SwaggerClient (see Issue #299).
The current (spec-compliant) behaviour remains with this new option as
it defaults to False. Overriding this default with 'True' means the path
element of the URL used to retrieve the spec will be used instead.