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 client response check after PR #2298 #2558

Merged
merged 4 commits into from
May 13, 2015

Conversation

vladlopes
Copy link
Contributor

The write endpoint is returning StatusNoContent instead of StatusOK when the write is successful.
This will be important only for clients that check for returning *Response being nil, as in both cases the err returned will be nil.

The write endpoint is returning StatusNoContent instead of StatusOK
when the write is successful.
@toddboom
Copy link
Contributor

@vladlopes good catch. however, you'll need to fix the associated tests before we can merge this. let me know if you're interested in doing that, otherwise one of us can take it over.

@vladlopes
Copy link
Contributor Author

I will take care of the tests...

The Client return a nil (json null) response on a sucessful write
@neonstalwart
Copy link
Contributor

nice... i couldn't figure out why {} started being returned when i changed the response to StatusNoContent in #2298. i'm glad you caught this.

@toddboom
Copy link
Contributor

awesome, kicked the build and we're green. 👍

@vladlopes
Copy link
Contributor Author

Maybe we should also check for a nil response on the client test (https://github.com/influxdb/influxdb/blob/master/client/influxdb_test.go#L116)? The test is discarding it...

@toddboom toddboom added this to the 0.9.0 milestone May 13, 2015
toddboom added a commit that referenced this pull request May 13, 2015
@toddboom toddboom merged commit b213388 into influxdata:master May 13, 2015
@toddboom
Copy link
Contributor

@vladlopes would you mind signing the CLA? Thanks!
http://influxdb.com/community/cla.html

@vladlopes
Copy link
Contributor Author

@toddboom Just did!

@vladlopes vladlopes deleted the fix-client-responsecheck branch May 14, 2015 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants