Skip to content

Update to gRPC 0.13.#1528

Merged
enisoc merged 7 commits into
vitessio:masterfrom
enisoc:grpc-0.13
Feb 27, 2016
Merged

Update to gRPC 0.13.#1528
enisoc merged 7 commits into
vitessio:masterfrom
enisoc:grpc-0.13

Conversation

@enisoc
Copy link
Copy Markdown
Contributor

@enisoc enisoc commented Feb 27, 2016

@alainjobart

They got rid of the alpha warnings for Python gRPC in 0.13.

Review on Reviewable

@enisoc enisoc force-pushed the grpc-0.13 branch 2 times, most recently from f0408b6 to 88d0d3a Compare February 27, 2016 01:02
@aaijazi
Copy link
Copy Markdown
Contributor

aaijazi commented Feb 27, 2016

LGTM

Approved with PullApprove

Comment thread travis/install_grpc.sh
pip install --root $grpc_dist --ignore-installed six
# Create a virtualenv, which also creates a virualenv-boxed pip.
virtualenv $grpc_dist/usr/local
$grpc_dist/usr/local/bin/pip install --upgrade --ignore-installed virtualenv tox six
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you do this to work-around the missing --root option for older pip binaries?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably more so to get rid of running pip install twice? (see below)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, I did it because it's the only way to get pip to see the updated six version. If you already have an older version of six installed via apt (likely a dependency of something else you installed, so you can't remove just python-six) then running regular pip will fail because it prefers the older six in system-wide dist-packages even though there's a newer one in /usr/local site-packages.

To force it to use the new one and ignore the system-wide one, you have to use virtualenv. I'm running into this same problem while trying to build the Docker bootstrap image. There we try to install grpc as a system package, but that's not possible because there's an older system python-six installed that's conflicting. Still looking for workarounds...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@michael-berlin found grpc/grpc#5111 (comment) which recommends side-loading pip to avoid the apt dependency on the old version of python-six. That worked for our jessie-based docker image.

@enisoc
Copy link
Copy Markdown
Contributor Author

enisoc commented Feb 27, 2016

The unit tests are failing for an unexpected reason: grpc/grpc-go#576

Apparently it can't handle error messages with newlines. I'm assuming this is a bug and not a new constraint, but in the meantime I'm trying to remove newlines from our errors wherever they're causing test failures. Interestingly, all integration tests were fine.

@enisoc enisoc force-pushed the grpc-0.13 branch 6 times, most recently from d97dccc to 8e9d6c8 Compare February 27, 2016 10:14
Anthony Yeh added 7 commits February 27, 2016 02:31
This is only necessary at the moment because gRPC 0.13 has a bug that
occurs when error messages contain newlines. However, we might as well
keep it this way because newlines in Go errors are not really
conventional anyway.
The pecl build isn't always up-to-date with the gRPC version we use.
@enisoc enisoc force-pushed the grpc-0.13 branch 2 times, most recently from 8e0cfda to bbbccb0 Compare February 27, 2016 11:01
enisoc pushed a commit that referenced this pull request Feb 27, 2016
@enisoc enisoc merged commit 9ced8cc into vitessio:master Feb 27, 2016
@enisoc enisoc deleted the grpc-0.13 branch February 27, 2016 11:14
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.

4 participants