Revert Google HTTP Client upgrade #1980
Conversation
honestly, I don't know, but it looks like jib-gradle-plugin doesn't need it. It must've happened during a rebase, but we can remove it and build still works. but jib-gradle-plugin also has no explicit dependency on guava or databind, so we can remove those too |
jib-core/build.gradle
Outdated
| implementation("com.google.http-client:google-http-client-apache-v2:${dependencyVersions.GOOGLE_HTTP_CLIENT_APACHE_V2}") { | ||
| exclude group: "org.apache.httpcomponents", module: "httpclient" | ||
| } | ||
| implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" | ||
| implementation("com.google.auth:google-auth-library-oauth2-http:${dependencyVersions.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP}") { | ||
| exclude group: "org.apache.httpcomponents", module: "httpclient" |
There was a problem hiding this comment.
does this still create a conflict?
There was a problem hiding this comment.
Just realized it pulls in google-http-client and upgrading 1.27.0 to 1.30.1, so I think I should exclude google-http-client.
+--- com.google.auth:google-auth-library-oauth2-http:0.16.2
| +--- com.google.auth:google-auth-library-credentials:0.16.2
| +--- com.google.http-client:google-http-client:1.30.1 (*)
| +--- com.google.http-client:google-http-client-jackson2:1.30.1
| | +--- com.google.http-client:google-http-client:1.30.1 (*)
There was a problem hiding this comment.
I wonder if that'll break at runtime then?
There was a problem hiding this comment.
https://github.com/googleapis/google-cloud-java/blob/v0.80.0/google-cloud-clients/pom.xml recommends
guava.version 26.0 (down from 28.0)
http-client.version 1.27.0 (from 1.31.0)
google.auth.version 0.12.0 (from 0.16.2)
I'll first trying excluding google-http-client from google-auth-library-oauth2-http rather than downgrading it. Will do some test if ADC works on GCB.
There was a problem hiding this comment.
ADC on GCB works at least.
jib-gradle-plugin/build.gradle
Outdated
| exclude group: "org.apache.httpcomponents", module: "httpclient" | ||
| } | ||
| implementation "org.apache.httpcomponents:httpclient:${dependencyVersions.APACHE_HTTP_CLIENT_OVERRIDE}" | ||
| implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" |
There was a problem hiding this comment.
you can remove this, and guava, and jackson
There was a problem hiding this comment.
Can there be any possibility that removing these is OK with compilation but causes trouble when the user actually uses our plugin by defining Jib in their builds, like not pulling in required dependencies at their run-time?
There was a problem hiding this comment.
those dependencies are inherited as runtime implementation from the parent modules.
There was a problem hiding this comment.
the reason dependencies still need to be jib-plugins-common is because that library doesn't bring in jib-core as a "sourceProject" (which is defined by us as included). This might be easier to explain in person though since it's all proprietary.
There was a problem hiding this comment.
although now that I look at it again, it's pretty dumb, jib-plugins-common shouldn't have to redefine those dependencies. There must be a better way, I'll fix that later.
There was a problem hiding this comment.
And as of now, jib-plugins-common defines google-http-client, guava, and jackson-databind but not google-auth-library-oauth2-http. Why is that?
There was a problem hiding this comment.
it's because it is defined as a regular project dependency (the gradle) way, it doesn't need to include google-auth.... because it doesn't use it.
a dependency defined as implementation on a project is exposed to dependency project as runtime. So they can't use it as a compile time dependency unless they redefine it. The way sourceProject works is that it takes anything defined as implementation and makes it available as implementation on the dependent project as well.
Given that this is confusing, it might be an area to improve the build.
|
I think this works. |
Fixes the regression of slow network operations introduced at Jib 1.5.0. It is supposed fix multiple issues described in #1970, #1946, #1917, and the Jib users group.
This PR reverts #1882 that upgraded the Google HTTP Client library. I started with
git revert, so the Java source file changes are not manual. (I just retained the most recent code comments.)About dependencies, I just downgraded Google HTTP Client and left others like Guava and the recent google-auth-library-oauth2-http. No particular reason.
@loosebazooka let's take a close look at the changes. And why is it that
jib-maven-plugin/build.gradledoesn't have the HTTP Client dependency and some others?jib-gradle-pluginandjib-plugins-commonhave it.I'll run integration test locally.