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

feat(v2/callctx): add new callctx package #291

Merged
merged 3 commits into from
Jun 23, 2023
Merged

Conversation

codyoss
Copy link
Member

@codyoss codyoss commented Jun 23, 2023

Add a new callctx package to gax to facilitate adding and retrieving values from context.Context that our libraries can use to share information throughout the call stack. The first addition to this package will be two APIs to allow users to set RPC headers that will be added to API requests our clients make.

A follow up PR will be made to add using these after these changes are merged with #290.

@codyoss codyoss requested a review from a team as a code owner June 23, 2023 16:08
@codyoss codyoss requested a review from tritone June 23, 2023 16:08
Add a new callctx package to gax to facilitate adding and
retrieving values from context.Context that our libraries can use
to share information throughout the call stack. The first addition
to this package will be two APIs to allow users to set RPC headers
that will be added to API requests our clients make.

A follow up PR will be made to add using these after these changes
are merged with googleapis#290.
@noahdietz
Copy link
Contributor

A follow up PR will be made to add using these after these changes are merged with 290.

Should we just submit this and update 290 to leverage these as well?

The way I see it, this PR provides users a transport-agnostic means of setting headers, but 290, the transport-specific helpers, need to still respect users that have set metadata.MD on the context (for both transports, because that's what we said to use for rest too...). So users can set metadata.MD OR they can use SetHeaders, but GAPIC should have one helper (per transport?) to pull all of these things together + add its own headers.

Copy link

@tritone tritone left a comment

Choose a reason for hiding this comment

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

Looks good, thank you Cody!

v2/callctx/callctx.go Outdated Show resolved Hide resolved
v2/callctx/callctx.go Outdated Show resolved Hide resolved
@codyoss codyoss added the automerge Merge the pull request once unit tests and other checks pass. label Jun 23, 2023
@codyoss codyoss merged commit 11503ed into googleapis:main Jun 23, 2023
4 checks passed
@codyoss codyoss deleted the callctx branch June 23, 2023 20:22
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 23, 2023
tritone added a commit to tritone/google-api-go-client that referenced this pull request Jul 5, 2023
This allows headers added to the context metadata to be applied to
the request. See googleapis/gax-go#291.
tritone added a commit to googleapis/google-api-go-client that referenced this pull request Jul 7, 2023
This allows headers added to the context metadata to be applied to
the request. See googleapis/gax-go#291.
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.

None yet

4 participants