Skip to content

deps: update gRPC to 1.14.0#4047

Merged
htuch merged 3 commits intoenvoyproxy:masterfrom
lizan:grpc_1.14
Aug 8, 2018
Merged

deps: update gRPC to 1.14.0#4047
htuch merged 3 commits intoenvoyproxy:masterfrom
lizan:grpc_1.14

Conversation

@lizan
Copy link
Copy Markdown
Member

@lizan lizan commented Aug 3, 2018

Signed-off-by: Lizan Zhou zlizan@google.com

Description:
Pick up latest gRPC 1.14 for partially local credentials support, and keep update with latest stable generally.

Risk Level: Low
Testing: CI
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Lizan Zhou <zlizan@google.com>
@htuch
Copy link
Copy Markdown
Member

htuch commented Aug 5, 2018

@lizan the CI tests fail, seems to be related to deps and envoy-filter-example. Can you investigate? Also, can you add to the commit comment any special motivation we have (e.g. is there a specific set of functions/features we're after, security rationale or just generally tracking latest?). Thanks.

@htuch htuch self-assigned this Aug 5, 2018
Signed-off-by: Lizan Zhou <zlizan@google.com>
def _com_github_grpc_grpc():
_repository_impl("com_github_grpc_grpc")

if "com_github_nanopb_nanopb" not in native.existing_rules():
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@htuch This is the reason for the previous CI error. I agree it looks kind of hacky but seems to be the best way to include it in Envoy today. Loading @com_google_grpc_grpc//bazel:grpc_deps.bzl and call grpc_deps() won't work at all in current structure we have in this file. Any better suggestion?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is a gRPC C core hard dependency, we can add it as an external dep, sure. We do this for other gRPC deps below, e.g. cares.

native.new_http_archive(
name = "com_github_nanopb_nanopb",
build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD",
strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b",
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.

should we move SHA into repository_locations.bzl?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, ideally we do this.

def _com_github_grpc_grpc():
_repository_impl("com_github_grpc_grpc")

if "com_github_nanopb_nanopb" not in native.existing_rules():
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is a gRPC C core hard dependency, we can add it as an external dep, sure. We do this for other gRPC deps below, e.g. cares.

native.new_http_archive(
name = "com_github_nanopb_nanopb",
build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD",
strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, ideally we do this.

Signed-off-by: Lizan Zhou <zlizan@google.com>
@qiwzhang
Copy link
Copy Markdown
Contributor

qiwzhang commented Aug 7, 2018

LGTM

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Rad.

@htuch htuch merged commit 797e824 into envoyproxy:master Aug 8, 2018
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