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

Add bound parameters map to Query object #7688

Merged
merged 2 commits into from
Dec 23, 2016
Merged

Add bound parameters map to Query object #7688

merged 2 commits into from
Dec 23, 2016

Conversation

harryrose
Copy link
Contributor

@harryrose harryrose commented Dec 5, 2016

Addresses #7687. Support for bound parameters was added in #6634. However, the golang client package was not updated to support the new behaviour. This PR extends the Query object to take a list of parameters, which are POSTed to the influxdb server when the query is executed.

  • CHANGELOG.md updated (No, as it is a trivial change which doesn't affect the server)
  • Rebased/mergable
  • Tests pass (See below)
  • Sign CLA (if not already signed)

Tests

I've added a test that the parameters are encoded, (which passes).

The following tests fail when I run go test -v ./.... However, this test is also failing with a clean check-out of the master repository, so I believe it is unrelated to this PR.

FAIL github.com/influxdata/influxdb/tsdb/engine/tsm1 21.017s

[tsm1] 2016/12/05 16:14:47 Snapshot for path /tmp/tsm497135727 written in 5.765042ms
--- FAIL: TestEngine_Backup (0.02s)
	engine_test.go:185: file name doesn't match:
			got: 000000002-000000001.tsm
			exp: /tmp/tsm497135727/000000001-000000001.tsm

@harryrose
Copy link
Contributor Author

This is probably redundant now, as influxdata/influxdb-client#3 has an implementation.

@jsternberg
Copy link
Contributor

Can you rebase this and update the changelog? That other repository isn't yet ready for primetime and it would be nice to offer bound parameters with the old API for those who want to use that.

@jsternberg jsternberg self-requested a review December 15, 2016 20:30
@harryrose
Copy link
Contributor Author

Hi. sorry it has taken me a while to get to this. I've rebased, and updated the changelog as requested.

@jsternberg jsternberg merged commit 0a04499 into influxdata:master Dec 23, 2016
@rkuchan rkuchan added this to the 1.2.0 milestone Dec 29, 2016
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.

3 participants