chore(generator): move _use_client_cert_effective to _compat.py.j2 - #17963
chore(generator): move _use_client_cert_effective to _compat.py.j2#17963hebaalazzeh wants to merge 7 commits into
_use_client_cert_effective to _compat.py.j2#17963Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the internal helper method _use_client_cert_effective from the client templates and replaces its calls with mtls.should_use_client_cert(). Additionally, all corresponding unit tests for the removed method have been cleaned up across the test templates and golden files. I have no feedback to provide as the changes are clean and consistent.
hebaalazzeh
changed the title
_use_client_cert_effective to _compat.py.j2
| """Returns whether client certificate should be used for mTLS.""" | ||
| if hasattr(mtls, "should_use_client_cert"): | ||
| return mtls.should_use_client_cert() | ||
| else: |
There was a problem hiding this comment.
I thought the pattern you were using for this file was to try importing from the helper library, and provide the backup implementation if there's an ImportError. Do you plan to use hasattr for everything instead now?
| else: | ||
| use_client_cert_str = os.getenv( | ||
| "GOOGLE_API_USE_CLIENT_CERTIFICATE", "false" | ||
| ).lower() |
There was a problem hiding this comment.
The mtls implementation is a bit different, using two env vars. Should we try to match here
There was a problem hiding this comment.
addressed matched the implementation
| raise EmptyUniverseError() | ||
| return resolved | ||
|
|
||
| def use_client_cert_effective() -> bool: |
There was a problem hiding this comment.
The mtls implementation is called should_use_client_cert. Should we use the same name here?
There was a problem hiding this comment.
addressed updated name
2 participants
We move
_use_client_cert_effectiveand the associated tests to_compat.py.j2. This will reduce duplication in packages that have multiple services and this can we removed in the future when we bump the minimum version of google-auth.