-
Notifications
You must be signed in to change notification settings - Fork 175
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
Added WithCustomBaseURL func #116
Conversation
Oops, travis is not passing because of the version test :I |
Fixed @marstr |
@@ -183,6 +183,16 @@ func WithBaseURL(baseURL string) PrepareDecorator { | |||
} | |||
} | |||
|
|||
// WithCustomBaseURL returns a PrepareDecorator that replaces brace-enclosed keys within the | |||
// request base URL (i.e., http.Request.URL) with the corresponding values from the passed map. | |||
func WithCustomBaseURL(baseURL string, urlParameters map[string]interface{}) PrepareDecorator { |
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.
Why does the map's value type need to be interface{}?
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.
Truth is it does not need to be interface. I'm following what WithPathParameters
and WithQueryParameters
do right now. Still will take a look if those can use map[string]string
.
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 is possible to gen map[string]string
and modify go-autorest to take them instead of map[string]interface{}
. The same goes for WithEscapedPathParameters()
, WithPathParameters()
and WithQueryParameters()
. What do you guys think about changing those too? I could update this PR and add a new one for AutoRest tomorrow.
cc @marstr
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.
Hmmm, I would support the above proposal to move them all to map[string]string
. Even better would be map[string]fmt.Stringer
Stringer Type documentation
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.
Nevermind, I retract the map[string]fmt.Stringer suggestion, I just played with it in go playground and don't like how that would impact declaring map literals.
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.
After digging into this more I understand the reasoning here, no need to change it.
* Added WithCustomBaseURL func * Bump version up * Fixing version test * Unfix version test
* 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
This is for
x-ms-parametrized-host
support.PTAL @jhendrixMSFT @marstr