Skip to content
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

fix: drop usage of distutils #541

Merged
merged 9 commits into from
Nov 2, 2023
Merged

fix: drop usage of distutils #541

merged 9 commits into from
Nov 2, 2023

Conversation

parthea
Copy link
Collaborator

@parthea parthea commented Oct 21, 2023

Fixes #507 🦕

  • Raises ValueError if the format of the GOOGLE_API_USE_CLIENT_CERTIFICATE is not as defined below
    (2) If GOOGLE_API_USE_CLIENT_CERTIFICATE environment variable
    is "true", then the ``client_cert_source`` property can be used
    to provide client certificate for mutual TLS transport. If
    not provided, the default SSL client certificate will be used if
    present. If GOOGLE_API_USE_CLIENT_CERTIFICATE is "false" or not
    set, no client certificate will be used.

This is the same behaviour that exists in gapic-generator-python here https://github.com/googleapis/gapic-generator-python/blob/45f18a69454bcff68c0f98535c68fbdb4e09db1d/gapic/templates/%25namespace/%25name_%25version/%25sub/services/%25service/client.py.j2#L253-L256

@parthea parthea requested review from a team as code owners October 21, 2023 13:24
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Oct 21, 2023
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a minor, non-blocking comment.

util.strtobool(os.getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false"))
)

use_client_cert = os.getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might as well us lower() in this assignment so you don't have to repeat it below

(It would be great to cast this to a bool like the previous code was doing, but it seems like too much overhead for the very limited use in a very compact LOC range. The string comparison is OK.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 614150d

@parthea parthea merged commit 4bd9e10 into main Nov 2, 2023
23 checks passed
@parthea parthea deleted the drop-usage-of-distutils branch November 2, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependency on deprecated distutils
4 participants