fix(http): honor and deprecate HttpxRequestAdapter's base_url argument - #761
Open
Max Azatian (HardMax71) wants to merge 1 commit into
Open
Max Azatian (HardMax71) wants to merge 1 commit into
Max Azatian (HardMax71) wants to merge 1 commit into
Conversation
Since 1.9.4 the adapter ignored its base_url argument without a word. Requests then failed with an unrelated URL error, or under a generated client went to the client's default server. An explicit base_url wins over the http client's again, as it did up to 1.9.3. Passing it now raises a DeprecationWarning that points to the http client or the base_url setter, since the other kiota adapters take the base URL from the client only. Fixes microsoft#501.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Overview
Since 1.9.4 (#481)
HttpxRequestAdapterignores itsbase_urlargument. With a plain adapter the request URL ends up relative, so the request fails with an error that never mentions the base URL: httpx's "Request URL is missing an 'http://' or 'https://' protocol." or the Azure provider's "Valid url scheme and host required". Under a generated client nothing fails. The client's constructor fills the emptybase_urlwith its default server, so requests go to that host instead of the one passed.An explicit
base_urlwins over the http client's again, as it did up to 1.9.3. Passing it raises a DeprecationWarning that points to the http client or thebase_urlsetter. #481 wanted the base URL to come from the client only, like the other kiota adapters, and kept the argument to avoid a breaking change. The warning gets users there, and nobody breaks before the argument goes away in a major version.Related Issue
Fixes #501
Related to #490, which asks to remove the argument. After this it can go in the next major version.
Notes
Neither Graph SDK passes
base_url. The GraphRequestAdapter in msgraph-sdk-python and msgraph-beta-sdk-python only passeshttp_client, so this doesn't touch microsoftgraph/msgraph-beta-sdk-python#743, which was fixed in the beta SDK itself (microsoftgraph/msgraph-beta-sdk-python@34444b94). The bundle's DefaultRequestAdapter doesn't pass it either.If you'd rather only warn and keep ignoring the argument, or honor it without the warning, either is a small change to this PR.
Testing Instructions
cd packages/http/httpx && pytest tests/test_httpx_request_adapter.py -k base_url: fix: only use base_uri from http client #481's test that asserted the argument is ignored now checks that it's used and warns, a new test checks that it wins over the http client's base URL, and the http client test passes no argument and checks there's no warning. The first two fail on main.