Skip to content

feat(gax): implement grpc client - #415

Open
nolanleastin wants to merge 4 commits into
googleapis:mainfrom
nolanleastin:neastin/grpc-client
Open

feat(gax): implement grpc client#415
nolanleastin wants to merge 4 commits into
googleapis:mainfrom
nolanleastin:neastin/grpc-client

Conversation

@nolanleastin

Copy link
Copy Markdown
Contributor

No description provided.

@nolanleastin
nolanleastin marked this pull request as ready for review August 12, 2026 23:02
@nolanleastin
nolanleastin requested a review from a team as a code owner August 12, 2026 23:02
@coryan coryan changed the title feat(wkt): implement grpc client feat(gax): implement grpc client Aug 12, 2026
@coryan

coryan commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

I changed the title to feat(gax): ...

@coryan coryan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The dependency stuff is a blocker for me, the other stuff is minor.

Comment thread packages/gax/Sources/GoogleCloudGax/GRPCClient.swift Outdated
if !routingParams.isEmpty {
callOptions.customMetadata.add(
name: "x-goog-request-params",
value: routingParams.joined(separator: "&")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume then routingParams has to be percent-encoded already? please document this in the parameters for this function.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still applicable, please document the restrictions on routingParams.

Comment thread packages/gax/Sources/GoogleCloudGax/GRPCClient.swift Outdated
Comment thread packages/gax/Package.swift Outdated

@coryan coryan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some nits, please fix and merge


/// Common Google header names used across HTTP and gRPC transports.
@_spi(GoogleCloudInternal)
public enum HeaderNames {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Belt and suspenders ...

Suggested change
public enum HeaderNames {
public enum _HeaderNames {

Ignore what I said about _apiClient one symbol with underscores is enough.

if !routingParams.isEmpty {
callOptions.customMetadata.add(
name: "x-goog-request-params",
value: routingParams.joined(separator: "&")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still applicable, please document the restrictions on routingParams.


import Foundation
import Testing
@_spi(GoogleCloudInternal) @testable import GoogleCloudGax

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe just

Suggested change
@_spi(GoogleCloudInternal) @testable import GoogleCloudGax
import GoogleCloudGax

??

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.

2 participants