Allow to pass and override default hard coded api version #274
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
…ersion Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
vbauzys
changed the title
lvirbalas
requested changes
Dec 9, 2019
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
dataclouder
reviewed
Dec 9, 2019
govcd/api.go
Outdated
| @@ -188,6 +196,12 @@ func (cli *Client) NewRequest(params map[string]string, method string, reqUrl ur | |||
| return cli.NewRequestWitNotEncodedParams(params, nil, method, reqUrl, body) | |||
| } | |||
|
|
|||
| // NewRequest creates a new HTTP request and applies necessary auth headers if set. | |||
There was a problem hiding this comment.
Suggested change
| // NewRequest creates a new HTTP request and applies necessary auth headers if set. | |
| // NewRequestWithApiVersion creates a new HTTP request and applies necessary auth headers if set. |
dataclouder
reviewed
Dec 9, 2019
| @@ -127,6 +129,12 @@ func ContainsNotFound(err error) bool { | |||
|
|
|||
| // NewRequestWitNotEncodedParams allows passing complex values params that shouldn't be encoded like for queries. e.g. /query?filter=name=foo | |||
| func (cli *Client) NewRequestWitNotEncodedParams(params map[string]string, notEncodedParams map[string]string, method string, reqUrl url.URL, body io.Reader) *http.Request { | |||
| return cli.NewRequestWitNotEncodedParamsWithApiVersion(params, notEncodedParams, method, reqUrl, body, cli.APIVersion) | |||
There was a problem hiding this comment.
I'd like to see a few tests for the new functions, to make sure that:
- They are really using the intended API version.
- Subsequent requests using the regular
Execute*methods are not affected.
There was a problem hiding this comment.
Could you share your idea how to test that?
There was a problem hiding this comment.
Here's a possible way:
- Add fields to a type that was enhanced in API versions later than the current one.
- Run a GET that retrieves the type with the enhanced fields
- Make sure the fields are non-empty
- Run the same query without the new API version
- Make sure that the enhanced fields are empty
There was a problem hiding this comment.
Two more ideas:
- https://golang.org/pkg/net/http/httptest/
or - IIRC when you send a request on a specific version, vCD responds with API version (in headers). You could probably check that.
There was a problem hiding this comment.
first option could be valid with on specific vCD version or I have to find valid for all versions, which is lottery
I'll check possibility with response headers
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
lvirbalas
approved these changes
Dec 9, 2019
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Didainius
reviewed
Dec 9, 2019
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com> # Conflicts: # CHANGELOG.md
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
Didainius
reviewed
Dec 9, 2019
|
|
||
| // Test_NewRequestWitNotEncodedParamsWithApiVersion verifies that api version override works | ||
| func (vcd *TestVCD) Test_NewRequestWitNotEncodedParamsWithApiVersion(check *C) { | ||
| fmt.Printf("Running: %s\n", check.TestName()) |
There was a problem hiding this comment.
Double check the outputs as they are missing new lines. Heres' how it looks.
START: api_test.go:105: TestVCD.Test_NewRequestWitNotEncodedParamsWithApiVersion
Running: TestVCD.Test_NewRequestWitNotEncodedParamsWithApiVersion
Test: TestVCD.Test_NewRequestWitNotEncodedParamsWithApiVersion run with api Version: 33.0
PASS: api_test.go:105: TestVCD.Test_NewRequestWitNotEncodedParamsWithApiVersion 0.191s
There was a problem hiding this comment.
Ok. This is fine as it is as planned.
Didainius
approved these changes
Dec 9, 2019
Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>
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 change is driven from VM internal disk PR(#272) and investigation. Also we saw need that in near feature in other cases, as we face more such cases when we need specific version due limitation of using lowest possible API version.
WithApiVersionwhich allows to pass API version which will be written in request header(override default hard coded one)We need override only per request base API version. Changing connection API version would create compatibility issues and breakage.