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

Update grpc and io_opencensus_cpp #2131

Closed
wants to merge 7 commits into from
Closed

Update grpc and io_opencensus_cpp #2131

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 3, 2019

See grpc/grpc#18080

Pinging @devjgm

The problem as mentioned in the above issue is that abseil moved the location of a file. As a result, the version of io_opencensus_cpp we are using breaks. We need to update it too.

This PR doesn't update protobuf per #2090 yet, because I'm not entirely sure if we need to.


This change is Reviewable

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 3, 2019
@codecov
Copy link

codecov bot commented Mar 3, 2019

Codecov Report

Merging #2131 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2131   +/-   ##
=======================================
  Coverage   92.87%   92.87%           
=======================================
  Files         300      300           
  Lines       17696    17696           
=======================================
  Hits        16435    16435           
  Misses       1261     1261

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 979c477...792b617. Read the comment docs.

@coryan coryan added the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 3, 2019
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 3, 2019
@coryan coryan added the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 3, 2019
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 3, 2019
@coryan coryan added the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 4, 2019
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 4, 2019
@coryan
Copy link
Contributor

coryan commented Mar 5, 2019

I am sad about using SHAs for the dependencies. Can we wait until gRPC and protobuf release a compatible version? Or can we just update the protobuf version and override the gRPC preference?

@coryan coryan requested review from coryan and devjgm March 5, 2019 13:18
@devjgm
Copy link
Contributor

devjgm commented Mar 5, 2019

I am sad about using SHAs for the dependencies. Can we wait until gRPC and protobuf release a compatible version? Or can we just update the protobuf version and override the gRPC preference?

There should be a new release of grpc that works as soon as grpc/grpc#18080 is resolved. And open census had the v0.3.0 release on Jan 31 (https://github.com/census-instrumentation/opencensus-cpp/releases). Would that released version work for us so we don't need to depend on a commit hash?

@ghost
Copy link
Author

ghost commented Mar 5, 2019

There is a PR to backport the fix to 1.19.x: grpc/grpc#18202

And the opencensus release should work because it refers to the new file location.

Copy link
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

I think we should wait until gRPC makes a new release with the fix you mentioned above. Thanks for you patience with this.

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @coryan and @devjgm)

@ghost ghost closed this Mar 8, 2019
@ghost ghost deleted the update-grpc-v2 branch July 23, 2019 19:28
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants