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

Adding keyspace to go client API. #1727

Merged
merged 3 commits into from May 25, 2016
Merged

Conversation

alainjobart
Copy link
Contributor

@alainjobart alainjobart commented May 24, 2016

It's a per connection parameter, so we don't have to send it in every
Execute() or StreamExecute() call.

@enisoc go version of the change.


This change is Reviewable

It's a per connection parameter, so we don't have to send it in every
Execute() or StreamExecute() call.
And fixing the bug it uncovered. No tests = bug for sure.
@enisoc
Copy link
Member

enisoc commented May 25, 2016

Reviewed 12 of 12 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


go/cmd/vtgateclienttest/goclienttest/main.go, line 27 [r1] (raw file):

  // Create a client connecting to the server
  ctx := context.Background()
  conn, err := vtgateconn.DialProtocol(ctx, protocol, addr, 30*time.Second, "conn_ks")

Need to add assertion of keyspace value for Execute(), StreamExecute(), and tx.Execute() in echo.go.


go/vt/vtctl/query.go, line 50 [r1] (raw file):

          "VtGateExecute",
          commandVtGateExecute,
          "-server <vtgate> [-bind_variables <JSON map>] [-connect_timeout <connect timeout>] [-tablet_type <tablet type>] [-json] <sql>",

Add [-keyspace <default keyspace>].

Also regenerate vtctl reference doc.


Comments from Reviewable

Also checking connection keyspace in go unit test.
Fixing vtctl doc, regenerating md file.
@alainjobart
Copy link
Contributor Author

Review status: 10 of 14 files reviewed at latest revision, 2 unresolved discussions.


go/cmd/vtgateclienttest/goclienttest/main.go, line 27 [r1] (raw file):

Previously, enisoc (Anthony Yeh) wrote…

Need to add assertion of keyspace value for Execute(), StreamExecute(), and tx.Execute() in echo.go.

Done.

go/vt/vtctl/query.go, line 50 [r1] (raw file):

Previously, enisoc (Anthony Yeh) wrote…

Add [-keyspace <default keyspace>].

Also regenerate vtctl reference doc.

Done.

Comments from Reviewable

@enisoc
Copy link
Member

enisoc commented May 25, 2016

:lgtm:

Previously, alainjobart (Alain Jobart) wrote…

Adding keyspace to go client API.

It's a per connection parameter, so we don't have to send it in every

Execute() or StreamExecute() call.

@enisoc go version of the change.


Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

Approved with PullApprove

@alainjobart alainjobart merged commit c4ca137 into vitessio:master May 25, 2016
@alainjobart alainjobart deleted the keyspace branch May 25, 2016 21:25
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.

None yet

3 participants