[python] Support proxy_ssl_context for the urllib3-based client - #24539
Open
emmanuel-adu wants to merge 4 commits into
Open
[python] Support proxy_ssl_context for the urllib3-based client#24539emmanuel-adu wants to merge 4 commits into
emmanuel-adu wants to merge 4 commits into
Conversation
Adds an optional Configuration.proxy_ssl_context so callers can supply a distinct SSL context for the TLS handshake with an HTTPS proxy, separate from the destination (API server) TLS settings, per urllib3's proxy_ssl_context support. Defaults to None (no behavior change). Applies to the python (urllib3 library) and python-pydantic-v1 generators. Not applicable to the SOCKS proxy path (no TLS handshake with a SOCKS proxy) or to the aiohttp-based async client (does not use urllib3's ProxyManager).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 19 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/echo_api/python-pydantic-v1/openapi_client/rest.py">
<violation number="1" location="samples/client/echo_api/python-pydantic-v1/openapi_client/rest.py:110">
P1: Clients resolved to supported urllib3 1.25.x fail when callers set this option, because that release does not implement `ProxyManager.proxy_ssl_context`. Raise the generated urllib3 minimum to 1.26.0 (including all package metadata/templates), or guard/reject the option for older versions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) | ||
| else: | ||
| if configuration.proxy_ssl_context is not None: | ||
| addition_pool_args['proxy_ssl_context'] = configuration.proxy_ssl_context |
Contributor
There was a problem hiding this comment.
P1: Clients resolved to supported urllib3 1.25.x fail when callers set this option, because that release does not implement ProxyManager.proxy_ssl_context. Raise the generated urllib3 minimum to 1.26.0 (including all package metadata/templates), or guard/reject the option for older versions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/echo_api/python-pydantic-v1/openapi_client/rest.py, line 110:
<comment>Clients resolved to supported urllib3 1.25.x fail when callers set this option, because that release does not implement `ProxyManager.proxy_ssl_context`. Raise the generated urllib3 minimum to 1.26.0 (including all package metadata/templates), or guard/reject the option for older versions.</comment>
<file context>
@@ -106,6 +106,8 @@ def __init__(self, configuration, pools_size=4, maxsize=None) -> None:
)
else:
+ if configuration.proxy_ssl_context is not None:
+ addition_pool_args['proxy_ssl_context'] = configuration.proxy_ssl_context
self.pool_manager = urllib3.ProxyManager(
num_pools=pools_size,
</file context>
- Configuration.__deepcopy__ special-cases proxy_ssl_context (like the existing retries/proxy_headers handling), since ssl.SSLContext cannot be deep-copied and previously raised TypeError. - Gate proxy_ssl_context to the true urllib3 library variant only (not asyncio/aiohttp, not the deprecated tornado library), since neither of those REST clients read the option. - Raise python-pydantic-v1's generated urllib3 floor from 1.25.3 to 1.26.0, the version that added ProxyManager's proxy_ssl_context kwarg, so setting the option can't crash under an allowed-but-too-old urllib3.
Author
|
Addressed all 7 findings in 2a30617:
All samples regenerated via `./bin/generate-samples.sh ./bin/configs/python*.yaml` and `./bin/utils/export_docs_generators.sh` (no doc diff, as expected). |
Member
|
thanks for the pr please review the build errors when you've time |
My previous restructuring of the non-useIndependentImplicitClients
__deepcopy__ branch (if k in (...): continue / setattr(...)) changed
the exact generated line
if k not in ('logger', 'logger_file_handler'):
that PythonClientCodegenTest.testIndependentImplicitClientsPreserveDefaultsWhenDisabled
asserts on verbatim, breaking CI on all 3 Java build jobs.
Restore that literal line unchanged and instead insert the
proxy_ssl_context special-case as an early continue before it, which
is behavior-identical and a smaller diff. Verified locally: the
previously-failing PythonClientCodegenTest now passes (62/62), and the
proxy_ssl_context deepcopy fix still works end-to-end.
Author
@wing328 addressed build error |
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
2 participants
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.
Description
Adds
Configuration.proxy_ssl_context(defaultNone, no behavior change) so callers can supply an SSL context used only for the TLS handshake with an HTTPS proxy, independent of the destination TLS settings. This maps directly to urllib3's ownproxy_ssl_contextsupport inProxyManager.Applies to the
python(urllib3 library) andpython-pydantic-v1generators. Not applicable to the SOCKS proxy path (no TLS handshake against a SOCKS proxy) or to the aiohttp-based async client (doesn't use urllib3'sProxyManager).Context: came up in kubernetes-client/python#2658, where this was requested to live in the upstream generator rather than as a post-processing patch downstream.
cc @cbornet @tomplus @krjakbrjak @fa0311 (Python technical committee)
PR checklist
./mvnw clean package,./bin/generate-samples.sh ./bin/configs/python*.yaml, and./bin/utils/export_docs_generators.sh(no doc diff, since this isn't a CLI/codegen option). All changed samples are committed.Summary by cubic
Adds
Configuration.proxy_ssl_contextto theurllib3Python client and forwards it toProxyManagerfor HTTPS proxy TLS handshakes.Defaults to
None; only applies to urllib3 (not SOCKS/aiohttp/tornado); fixesConfiguration.__deepcopy__forssl.SSLContextwhile preserving generator behavior; bumpspython-pydantic-v1urllib3floor to>=1.26.0.Written for commit b0ed61c. Summary will update on new commits.