Adding ByDiscardingBody() decorator. #118
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
This issue is detailed in Azure#117
|
Question, would this func be included in all Responder funcs in the SDK, or just in the ones that do not use the response body? |
|
Just ones that don't use the body. Though, in theory it shouldn't hurt to add it everywhere. |
mcardosos
reviewed
Feb 14, 2017
autorest/version.go
Outdated
| @@ -7,7 +7,7 @@ import ( | |||
| const ( | |||
| major = "7" | |||
| minor = "2" | |||
There was a problem hiding this comment.
I think it is not a patch, but a minor version.
There was a problem hiding this comment.
Ah, good point. :) I'll fix that up.
mcardosos
approved these changes
Feb 14, 2017
| @@ -21,6 +21,7 @@ and Responding. A typical pattern is: | |||
| DoRetryForAttempts(5, time.Second)) | |||
|
|
|||
| err = Respond(resp, | |||
| ByDiscardingBody(), | |||
There was a problem hiding this comment.
I see this is in a code block, probably not a big deal.
There was a problem hiding this comment.
+1
yeah, code in markdown is funky, especially in PRs.
autorest/responder_test.go
Outdated
| @@ -198,7 +202,8 @@ func TestByCopying_AcceptsNilBody(t *testing.T) { | |||
|
|
|||
| func TestByClosing(t *testing.T) { | |||
| r := mocks.NewResponse() | |||
| err := Respond(r, ByClosing()) | |||
| err := Respond(r, | |||
| ByClosing()) | |||
There was a problem hiding this comment.
Was this supposed to be ByDiscardingBody or was the line break intentional?
There was a problem hiding this comment.
I had added ByDiscardingBody() while on auto-pilot applying my fix, but thought better of it because this test should isolate ByClosing(). I'll fix up this formatting so that it doesn't jump out to folks looking at diffs later.
ccojocar
pushed a commit
to ccojocar/go-autorest
that referenced
this pull request
Feb 24, 2017
* Adding ByDiscardingBody() decorator. This issue is detailed in Azure#117 * Updating reported version number. * Responding to PR feedback.
marstr
pushed a commit
that referenced
this pull request
Mar 23, 2017
* Extract the authentication code in a separate package 'adal' * Update the log prefix to reflect the package path * Move the authorizer in its own file * Remove the dependency of service principal token on autorest package * Remove the dependency of device code flow on autorest package * Move the bearer authorizer in autorest package Introduce new interfaces for token provider and refresh functions. * Fix the example application * Fix the style of imports * Remove UTF-8 BOM from response body * Changelog (#114) * Added changelog for v7.2.4 * Ran glide up * Addimg BOM change * Correct version number * Adding ByDiscardingBody() decorator. (#118) * Adding ByDiscardingBody() decorator. This issue is detailed in #117 * Updating reported version number. * Responding to PR feedback. * Updating dependencies. * Adding ACR suffix to pulic cloud env (#104) * Adding ACR suffix to pulic cloud env (USgov, China and Germany still missing) * Added container registry DNS suffix in all envs * Added WithCustomBaseURL func (#116) * Added WithCustomBaseURL func * Bump version up * Fixing version test * Unfix version test * Updating change-log for v7.3.0 release. * Setting the current version to 8.0.0 * Add the ContentLength header in requests * Fix the build * Convert into a constant the Active Directory endpoint template * Read the body of HTTP response before checking its status * Use the status codes provided by Go HTTP library in device code test * Use the status code provide by Go HTTP library in the token test * Use the status codes provide by HTTP Go library in the authorization tests * Add a generic command line tool for adal library. This tool is based on the example from azure package. It is a bit more generic it the sense that one can retrieve a token for a given resource. * Add a README file for Azure Active Directory library for Go The documentation is based on the Readme file from azure/exmaple but the configuration steps were update for Azure CLI 2.0. In addition some code snippets for each authentication flow were added. * Update the error messages to use status codes provided by Go HTTP library
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.
This issue is detailed in #117