Conversation
yamaceay
changed the title
560ab04 to
96ae5c6
Compare
yamaceay
added
duplicate
0884456 to
536fa3f
Compare
536fa3f to
313b78d
Compare
|
The script that was used to migrate to HTTPX2 is attached below |
yamaceay
removed
don't merge
duplicate
alpkom
left a comment
alpkom
left a comment
There was a problem hiding this comment.
Since we're already touching user interface (even though minimal), let's get rid of any reference to httpx/httpx2 from user facing functions/initializers, so that we don't need breaking changes later on.
| message: str, | ||
| status_code: int, | ||
| headers: httpx.Headers, | ||
| headers: httpx2.Headers, |
There was a problem hiding this comment.
Since we're already changing the user facing type, let's change it to simple dict, in order to avoid disclosing implementation details to users and to avoid breaking changes in the future.
| :param timeout: Default HTTP request timeout passed to httpx2. | ||
| :type timeout: Union[int, float, httpx2.Timeout], Optional |
There was a problem hiding this comment.
Let's not refer to httpx2 at all, and don't accept httpx2.Timeout as type.
| proxy_client: Optional[GenAIHubProxyClient] = None, | ||
| resource_group: Optional[str] = None, | ||
| timeout: Union[int, float, httpx.Timeout, None] = None, | ||
| timeout: Union[int, float, httpx2.Timeout, None] = None, |
There was a problem hiding this comment.
Let's not accept httpx2.Timeout as type at all
| provider: str, | ||
| model: str, | ||
| timeout: Union[int, float, httpx.Timeout, None] = None, | ||
| timeout: Union[int, float, httpx2.Timeout, None] = None, |
There was a problem hiding this comment.
Let's not accept httpx2.Timeout as type at all
| :type model: str | ||
| :param timeout: Per-request timeout override. | ||
| :type timeout: Union[int, float, httpx.Timeout], Optional | ||
| :type timeout: Union[int, float, httpx2.Timeout], Optional |
There was a problem hiding this comment.
Let's not accept httpx2.Timeout as type at all
| model_name: Optional[str] = None, | ||
| model_version: Optional[str] = None, | ||
| timeout: Union[int, float, httpx.Timeout, None] = None, | ||
| timeout: Union[int, float, httpx2.Timeout, None] = None, |
There was a problem hiding this comment.
Let's not accept httpx2.Timeout as type at all
| Can be an integer, float, or an instance of `httpx2.Timeout`. | ||
| :type timeout: Union[int, float, httpx2.Timeout, None] |
There was a problem hiding this comment.
Let's not accept httpx2.Timeout as type at all
| """ | ||
| Shared SSL context factory for httpx2 clients. | ||
|
|
||
| httpx2 defaults to the OS trust store (truststore). This module preserves | ||
| the pre-migration behaviour of using certifi's CA bundle so existing | ||
| deployments are not affected. Switch callers to verify=True to adopt the | ||
| httpx2 default when ready. | ||
| """ | ||
| import ssl | ||
| import certifi | ||
|
|
||
|
|
||
| def default_ssl_context() -> ssl.SSLContext: | ||
| """Return an SSL context backed by certifi's CA bundle.""" | ||
| return ssl.create_default_context(cafile=certifi.where()) |
There was a problem hiding this comment.
Have you checked if setting this default SSL Context is needed for our use case? Do the tests fail if this is not defined, and default of httpx2 is used instead?
| dependencies = [ | ||
| "httpx>=0.27.0", | ||
| "httpx2>=2.0.0", | ||
| "certifi", |
There was a problem hiding this comment.
Let's specify a version here. (not ==, but >=)
| "pylint==4.0.8", | ||
| "requests-mock==1.12.1", | ||
| "respx==0.23.1", | ||
| "httpx2-pytest", |
There was a problem hiding this comment.
Let's specify a version here
This branch has not been deployed
3 participants
What this PR does and why it is needed
This PoC migrates
packages/gen/from HTTPX to HTTPX2 directly. Major release needed since HTTPX is entirely removed from the gen package.Definition of Done