Skip to content
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

fix: Allow connecting to a server on a URL path (#149) #152

Merged
merged 3 commits into from
Jul 15, 2020

Conversation

vlastahajek
Copy link
Contributor

Closes #149

Properly concatenating URL path to the final API endpoint

  • CHANGELOG.md updated
  • Rebased/mergeable
  • Tests pass

@vlastahajek vlastahajek requested a review from sranka July 14, 2020 13:36
@vlastahajek vlastahajek changed the title fix: Allow connecting to a server on an URL path (#149) fix: Allow connecting to a server on a URL path (#149) Jul 14, 2020
{"http://host:9999/", "http://host:9999/api/v2/", "http://host:9999/api/v2/write"},
{"http://host:9999/path", "http://host:9999/path/api/v2/", "http://host:9999/path/api/v2/write"},
{"http://host:9999/path/", "http://host:9999/path/api/v2/", "http://host:9999/path/api/v2/write"},
{"http://host:9999/path1/path2/path3", "http://host:9999/path1/path2/path3/api/v2/", "http://host:9999/path1/path2/path3/api/v2/write"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line (and the next one, which is the same) is IMHO redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/path1/path2/path3 is tested as a more complex path and they differ in the '/' at the end

@@ -42,7 +71,7 @@ func TestServerError429(t *testing.T) {
w.Header().Set("Retry-After", "1")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusTooManyRequests)
w.Write([]byte(`{"code":"too many requests", "message":"exceeded rate limit"}`))
_, _ = w.Write([]byte(`{"code":"too many requests", "message":"exceeded rate limit"}`))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these blanks? Probably a violation of https://staticcheck.io/docs/checks#S1005

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly noting about ignoring returned val and returned error recommended by the style check in the IDE.
It's validated by running full profile staticcheck ('staticcheck -show-ignored -checks="all" ./...') and this case is ok

func TestServerErrorNonJSON(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
time.Sleep(100 * time.Millisecond)
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(`internal server error`))
_, _ = w.Write([]byte(`internal server error`))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -79,7 +124,7 @@ func TestServerErrorInflux1_8(t *testing.T) {
w.Header().Set("X-Influxdb-Error", "bruh moment")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(404)
w.Write([]byte(`{"error": "bruh moment"}`))
_, _ = w.Write([]byte(`{"error": "bruh moment"}`))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vlastahajek vlastahajek merged commit aba0a86 into influxdata:master Jul 15, 2020
@vlastahajek vlastahajek deleted the fix/ulr-prefix branch July 15, 2020 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding URL prefix to client
2 participants