-
Notifications
You must be signed in to change notification settings - Fork 3.9k
core, inprocess, util: move inprocess and util code into their own new artifacts grpc-inprocess and grpc-util #10362
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
Changes from all commits
743edeb
0abd922
ccd6a4b
0b30c31
50f9552
ab7a652
662884b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,19 +6,8 @@ java_library( | |
|
||
java_library( | ||
name = "inprocess", | ||
srcs = glob([ | ||
"src/main/java/io/grpc/inprocess/*.java", | ||
]), | ||
visibility = ["//visibility:public"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. internal should not be public. Why did you need this? It was already allowing subpackages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
deps = [ | ||
":internal", | ||
"//api", | ||
"//context", | ||
"@com_google_code_findbugs_jsr305//jar", | ||
"@com_google_errorprone_error_prone_annotations//jar", | ||
"@com_google_guava_guava//jar", | ||
"@com_google_j2objc_j2objc_annotations//jar", | ||
], | ||
exports = ["//inprocess"], | ||
) | ||
|
||
java_library( | ||
|
@@ -47,21 +36,8 @@ java_library( | |
|
||
java_library( | ||
name = "util", | ||
srcs = glob([ | ||
"src/main/java/io/grpc/util/*.java", | ||
]), | ||
resources = glob([ | ||
"src/bazel-util/resources/**", | ||
]), | ||
visibility = ["//visibility:public"], | ||
deps = [ | ||
":internal", | ||
"//api", | ||
"@com_google_code_findbugs_jsr305//jar", | ||
"@com_google_guava_guava//jar", | ||
"@com_google_j2objc_j2objc_annotations//jar", | ||
"@org_codehaus_mojo_animal_sniffer_annotations//jar", | ||
], | ||
exports = ["//util"], | ||
) | ||
|
||
# Mirrors the dependencies included in the artifact on Maven Central for usage | ||
|
@@ -71,9 +47,8 @@ java_library( | |
name = "core_maven", | ||
visibility = ["//visibility:public"], | ||
exports = [ | ||
":inprocess", | ||
":internal", | ||
":util", | ||
"//api", | ||
"//inprocess", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should have exported //util, not //inprocess (to mirror core/build.gradle) |
||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1 @@ | ||
io.grpc.internal.PickFirstLoadBalancerProvider | ||
io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RR moving to grpc-util could definitely break users in a hard-to-diagnose way. I'm not sure how we want to handle this. Short-term we could have a runtimeDependency in grpc-core onto grpc-util. That would mean we'd also want There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Interesting. grpc-util has an api dependency on grpc-core so doing what you suggest makes the dependency circular (even with runtimeOnly dependency). Any ideas? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this is a blocker and needs to be resolved. So I'll wait for that before I address the remaining comments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was aware that would be the case. But it should work. Are you saying it doesn't work? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I had tried it before I added the comment:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been resolved There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hi i just wanted to let you know that this did indeed break for us in a hard to diagnose way. Is there a correct way to upgrade this library so that it does not break? in general we use something like ManagedChannelBuilder
.forTarget(URL)
.defaultLoadBalancingPolicy("round_robin")
.usePlaintext()
.build() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mdering "break" is not a helpful description. Please open a new issue and describe the error/problem you see when upgrading. You can link to this PR. For this specific change, it's also important whether you are shading/shadowing/making a fat jar. |
||
io.grpc.util.OutlierDetectionLoadBalancerProvider |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
java_library( | ||
name = "inprocess", | ||
srcs = glob([ | ||
"src/main/java/io/grpc/inprocess/*.java", | ||
]), | ||
visibility = ["//visibility:public"], | ||
deps = [ | ||
"//core:internal", | ||
"//api", | ||
"//context", | ||
"@com_google_code_findbugs_jsr305//jar", | ||
"@com_google_errorprone_error_prone_annotations//jar", | ||
"@com_google_guava_guava//jar", | ||
"@com_google_j2objc_j2objc_annotations//jar", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
plugins { | ||
id "java-library" | ||
id "maven-publish" | ||
|
||
id "ru.vyarus.animalsniffer" | ||
} | ||
|
||
description = 'gRPC: Inprocess' | ||
|
||
dependencies { | ||
api project(':grpc-core') | ||
|
||
implementation libraries.guava | ||
testImplementation project(':grpc-testing'), | ||
testFixtures(project(':grpc-core')) | ||
testImplementation libraries.guava.testlib | ||
|
||
signature libraries.signature.java | ||
signature libraries.signature.android | ||
} | ||
|
||
tasks.named("javadoc").configure { | ||
exclude 'io/grpc/inprocess/Internal*' | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
java_library( | ||
name = "util", | ||
srcs = glob([ | ||
"src/main/java/io/grpc/util/*.java", | ||
]), | ||
resources = glob([ | ||
"src/main/resources/**", | ||
]), | ||
visibility = ["//visibility:public"], | ||
deps = [ | ||
"//api", | ||
"//core:internal", | ||
"@com_google_code_findbugs_jsr305//jar", | ||
"@com_google_guava_guava//jar", | ||
"@com_google_j2objc_j2objc_annotations//jar", | ||
"@org_codehaus_mojo_animal_sniffer_annotations//jar", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
plugins { | ||
id "java-library" | ||
id "maven-publish" | ||
|
||
id "me.champeau.jmh" | ||
id "ru.vyarus.animalsniffer" | ||
} | ||
|
||
description = 'gRPC: Util' | ||
|
||
dependencies { | ||
api project(':grpc-core') | ||
|
||
implementation libraries.animalsniffer.annotations, | ||
libraries.guava | ||
runtimeOnly libraries.gson // to fix checkUpperBoundDeps error in services | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this should go in services. |
||
testImplementation testFixtures(project(':grpc-api')), | ||
testFixtures(project(':grpc-core')), | ||
project(':grpc-testing') | ||
testImplementation libraries.guava.testlib | ||
|
||
jmh project(':grpc-testing') | ||
|
||
signature libraries.signature.java | ||
signature libraries.signature.android | ||
} | ||
|
||
animalsniffer { | ||
// Don't check sourceSets.jmh | ||
sourceSets = [ | ||
sourceSets.main, | ||
sourceSets.test | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider | ||
io.grpc.util.OutlierDetectionLoadBalancerProvider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave a forwarding library in place. Keep
name
andvisibility
and have it export //inprocess. Ditto for util.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done