Skip to content

Conversation

@kartikaysaxena
Copy link
Contributor

Related #206

Implemented basic _createSecureConnector and made callCreds private to fix broken stuff (this PR), tested locally, also updates example

@kartikaysaxena kartikaysaxena requested a review from a team as a code owner April 1, 2025 16:51
@tstirrat15
Copy link
Contributor

I'll have a look at this soon - I do want to get this fixed. I don't fully understand how composition of these Credentials is supposed to work, and I'm a little worried that the code as it currently stands will whack the default rather than adding to a chain if additional CallCredentials are supplied. I need to spend a bit more time reading up on this stuff.

Copy link
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

See comments - otherwise this looks good to me. Thanks for putting this through!

Comment on lines 31 to 35

_createSecureConnector(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_createSecureConnector(
// NOTE: this is copied from the InsecureChannelCredentialsImpl class
_createSecureConnector(

Copy link
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>
@kartikaysaxena
Copy link
Contributor Author

updates the imports

@tstirrat15 tstirrat15 merged commit d75f8e7 into authzed:main Apr 14, 2025
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2025
@kartikaysaxena kartikaysaxena deleted the fix-util branch April 14, 2025 18:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants