Skip to content

Converting a few python clients to gRPC.#1484

Merged
alainjobart merged 4 commits into
vitessio:masterfrom
alainjobart:resharding
Feb 5, 2016
Merged

Converting a few python clients to gRPC.#1484
alainjobart merged 4 commits into
vitessio:masterfrom
alainjobart:resharding

Conversation

@alainjobart
Copy link
Copy Markdown
Contributor

@enisoc @sougou @aaijazi

@enisoc Only one left is the guestbook app, as far as I can tell. It seems like an easy fix, but I don't have the test environment to make it. Do you mind looking at it? Thanks!

v3 demo can now use grpc, but it doesn't work as v3's keyrange
construct produces invalid utf-8 strings. Will have to fix that.
Just use the v2 API, for debug purposes. Also now using grpc.
@alainjobart
Copy link
Copy Markdown
Contributor Author

@dumbunny please look at vtgate_client.py changes.

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Feb 4, 2016

LGTM for v3 changes.

Approved with PullApprove

@michael-berlin
Copy link
Copy Markdown
Contributor

Will the v3 demo change be visible in the demo UI? If not, I would prefer to have an indication of that.

(Before this change, the queries matched very well with what was actually expected to happen.)

@aaijazi
Copy link
Copy Markdown
Contributor

aaijazi commented Feb 4, 2016

reviewed it, generally looks fine, but going to defer to others for their expertise.

@alainjobart
Copy link
Copy Markdown
Contributor Author

@michael-berlin there is no visible change to the v3 demo. Only the queries that were done internally to show the content of each shard were affected, as expected. So I don't think it's changing anything.

@enisoc
Copy link
Copy Markdown
Contributor

enisoc commented Feb 5, 2016

LGTM I'll update the guestbook app after you merge.

Approved with PullApprove

alainjobart added a commit that referenced this pull request Feb 5, 2016
Converting a few python clients to gRPC.
@alainjobart alainjobart merged commit f256549 into vitessio:master Feb 5, 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.

6 participants