-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chore(generator): move _use_client_cert_effective to _compat.py.j2
#17963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
4ca8ee4
74ec086
0dcc129
285b8f5
e64984d
fdb2ef8
3279a6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,10 +13,13 @@ falling back to the local implementation if not present. #} | |
| {# TODO(https://github.com/googleapis/google-cloud-python/issues/17883): | ||
| Backfill compatibility functions being removed from the client layer. #} | ||
|
|
||
| import os | ||
|
|
||
| from typing import Optional | ||
| from urllib.parse import urlparse, urlunparse | ||
|
|
||
| from google.auth.exceptions import MutualTLSChannelError | ||
| from google.auth.transport import mtls | ||
| from google.api_core.universe import EmptyUniverseError | ||
|
|
||
| {% if has_auto_populated_fields %} | ||
|
|
@@ -142,6 +145,21 @@ def get_universe_domain( | |
| raise EmptyUniverseError() | ||
| return resolved | ||
|
|
||
| def use_client_cert_effective() -> bool: | ||
| """Returns whether client certificate should be used for mTLS.""" | ||
| if hasattr(mtls, "should_use_client_cert"): | ||
| return mtls.should_use_client_cert() | ||
| else: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| use_client_cert_str = os.getenv( | ||
| "GOOGLE_API_USE_CLIENT_CERTIFICATE", "false" | ||
| ).lower() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mtls implementation is a bit different, using two env vars. Should we try to match here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed matched the implementation |
||
| if use_client_cert_str not in ("true", "false"): | ||
| raise ValueError( | ||
| "Environment variable `GOOGLE_API_USE_CLIENT_CERTIFICATE` must be" | ||
| " either `true` or `false`" | ||
| ) | ||
| return use_client_cert_str == "true" | ||
|
|
||
| {% if has_auto_populated_fields %} | ||
|
|
||
| def setup_request_id( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mtls implementation is called
should_use_client_cert. Should we use the same name here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed updated name