Skip to content

Conversation

@clundin25
Copy link
Contributor

No description provided.

@clundin25 clundin25 requested review from a team as code owners October 30, 2023 21:41
@clundin25 clundin25 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 31, 2023
@clundin25 clundin25 added owlbot:run Add this label to trigger the Owlbot post processor. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Nov 30, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 30, 2023
Copy link
Contributor

@arithmetic1728 arithmetic1728 left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments.

self._enterprise_cert_file_path.encode("ascii"),
):
raise exceptions.MutualTLSChannelError(
"failed to configure SSL context"
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe update the error message to say which approach was used, something like "failed to configure SSL context using provider lib" / "failed to configure SSL context using offload lib". This will make it easier to identify issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, added!

self._enterprise_cert_file_path = enterprise_cert_file_path
self._cert = None
self._sign_callback = None
self._provider_lib = None
Copy link
Contributor

Choose a reason for hiding this comment

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

should we keep the following lines?

self._cert = None
self._sign_callback = None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, that was a mistake. I'll add those back.

@clundin25 clundin25 added kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Nov 30, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 30, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 30, 2023
@clundin25 clundin25 changed the title feat: Migrate custom tls signer to ECP Provider. feat: Add custom tls signer for ECP Provider. Nov 30, 2023
@clundin25 clundin25 added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 30, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 30, 2023
@clundin25 clundin25 merged commit 39eb287 into googleapis:main Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants