This repository was archived by the owner on Sep 26, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: enable setting quota_project_id #1128
feat: enable setting quota_project_id #1128
Changes from all commits
8e7b92f
c641572
cea6118
a157f44
1151fdc
c6b9d47
0a8d179
a63ffae
74541a5
6a56e6b
0e1e201
7f91763
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between headers and internal headers? And is it possible for quota project to come in on either one?
Also, what is the use case for pulling it from the header? I think maybe it is not necessary to detect this (@broady ?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quota project id can be gotten from header and internal header. double confirm with @chingor13
but I am not sure the use case. @broady
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the internal header provider is reserved for gapic and handwritten layers to inject headers. The general header provider is to allow library users to specify arbitrary extra headers.
It's not desired for library users to set the quota project via header provider, but previously that was the only mechanism for doing so. We should have an explicit strategy for how to handle that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems ok to support this then, for backwards compatibility. And also to ensure any quota project behavior stays consistent regardless of how it's set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chingor13 is it necessary to support setting quota project automatically through the incoming headers? This is not something I am familiar with, or something done in any other languages AFAIK. It also makes the precedent of these operations less intuitive.