Response header hooks #689
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I can see that the "errcheck" lint has failed, but the failure doesn't seem to have generated any information about the source location of the problem. 🤔 It looks like the job is intended to generate annotations for the source code to mark where the problems are -- I see it asking the tool to generate its results in a GitHub Actions-specific format -- but no annotations have appeared. I'm not sure what exactly that lint failure is referring to. |
brandonc
previously approved these changes
Apr 26, 2023
apparentlymart
force-pushed
the
matkins/response-header-hook
branch
from
689c150 to
7e3809b
Compare
The API for this client library is focused on returning the primary data associated with each API response, which typically means just the body of the response. Sometimes clients will also need to react to cross-cutting metadata such as expiration times, cache control guidance, and rate limiting information, which isn't a direct part of the data being requested but can nonetheless affect the behavior of the client. This information is typically returned in HTTP response header fields. To give access to this information without a breaking change to the API, this uses the context.Context API to allow the rare caller that needs it to register a hook through which it will be notified about the response header in any case where the request succeeded enough for there to be one. Most clients will not need this facility, which justifies the light abuse of the context.Context API for passing in this optional hook, even though this isn't the sort of cross-cutting concern context.Context should typically be used for.
apparentlymart
force-pushed
the
matkins/response-header-hook
branch
from
7e3809b to
0f0b3e2
Compare
brandonc
approved these changes
May 3, 2023
|
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The API for this client library is focused on returning the primary data associated with each API response, which typically means just the body of the response.
Sometimes clients will also need to react to cross-cutting metadata such as expiration times, cache control guidance, and rate limiting information, which isn't a direct part of the data being requested but can nonetheless affect the behavior of the client. This information is typically returned in HTTP response header fields.
To give access to this information without a breaking change to the API, this uses the
context.ContextAPI to allow the rare caller that needs it to register a hook through which it will be notified about the response header in any case where the request succeeded enough for there to be one.Most clients will not need this facility, which justifies the light abuse of the
context.ContextAPI for passing in this optional hook, even though this isn't the sort of cross-cutting concerncontext.Contextshould typically be used for.The goal of introducing this now is to improve hashicorp/terraform#32983 so that the Terraform Cloud / Enterprise API can return a new response header signaling its preferred state snapshot interval, thereby creating a compromise between creating no intermediate snapshots at all vs. creating so many intermediate snapshots as to overwhelm the storage of Terraform Enterprise customers and run across potential race conditions in Terraform Cloud.
This is a compromise to have a sort of relief valve in case of problems. This does not necessarily replace other efforts to store and present intermediate snapshots in a better way in Terraform Cloud, although it could potentially allow working on those with less urgency.