-
Notifications
You must be signed in to change notification settings - Fork 243
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
Remove *http.Response returns from API methods #305
Milestone
Comments
So I think I came up with a pattern we can use that allows us to remove this from the majority of the API, while also giving consumers a debug capability to allow them to capture the response if they need it. I'll raise a PR for that once v1.4.0 is out. |
theckman
added a commit
that referenced
this issue
Apr 23, 2021
In prepration for #305, this adds a mechanism to inspect all API responses received by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release.
theckman
added a commit
that referenced
this issue
Apr 23, 2021
In prepration for #305, this adds a mechanism to inspect all API responses received by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some deduplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
added a commit
that referenced
this issue
May 8, 2021
In prepration for #305, this adds a mechanism to inspect all API responses received by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some deduplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
added a commit
that referenced
this issue
May 29, 2021
In prepration for #305, this adds a mechanism to inspect all API responses received by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some deduplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
added a commit
that referenced
this issue
May 29, 2021
In prepration for #305, this adds a mechanism to inspect all API responses received by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some deduplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
added a commit
that referenced
this issue
Jul 4, 2021
In prepration for #305, this adds a mechanism to inspect all API requsts and responses handled by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field/feature, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some deduplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
added a commit
that referenced
this issue
Jul 5, 2021
In prepration for #305, this adds a mechanism to inspect all API requsts and responses handled by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field/feature, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some deduplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
added a commit
that referenced
this issue
Jul 5, 2021
In prepration for #305, this adds a mechanism to inspect all API requests and responses handled by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field/feature, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some duplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
So to my comment above, that was #325 which is now merged. I can start looking at this, finally. |
theckman
changed the title
Explore removing *http.Response returns from API methods
Remove *http.Response returns from API methods
Sep 4, 2021
theckman
added a commit
that referenced
this issue
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this issue
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this issue
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this issue
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this issue
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
This was
linked to
pull requests
Sep 4, 2021
This was
unlinked from
pull requests
Sep 4, 2021
This is done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we have some methods that return an
*httpResponse
, like this one:go-pagerduty/ruleset.go
Line 170 in 1a0c44e
Although, digging further I noticed that in many cases we read the response body and don't replace it:
go-pagerduty/ruleset.go
Line 174 in 1a0c44e
go-pagerduty/ruleset.go
Line 202 in 1a0c44e
go-pagerduty/client.go
Lines 314 to 318 in 1a0c44e
So, the only thing consumers can read from the
*http.Response
are the headers, and not the response. We should just remove this as one of the breaking changes in v1.5.0, since it never really worked.Impacted Files
Impacted Methods
The text was updated successfully, but these errors were encountered: