google-pub-sub-grpc, google-cloud-bigquery-storage: Bump pubsub protos and protobuf-java dep

[dvgica]

These protos were a couple years old, and so if you happen to also include newer GCP Java libs on your classpath, you can easily get conflicts between the class files generated by Pekko Connectors and those. This version is not quite the newest... unfortunately any newer version gives the following error which I'm disinclined to troubleshoot right now:

[error] /Users/dvangeest/projects/incubator-pekko-connectors/google-cloud-pub-sub-grpc/target/scala-2.13/pekko-grpc/main/com/google/protobuf/descriptor/FeatureSet.scala:301:21: illegal cyclic reference involving type EnumType
[error]     type EnumType = EnumType
[error]                     ^
[error] one error found

Also, depending on protobuf-java seems unnecessary?

[pjfanning]

Can you remove the protobuf-java changes? Protobuf-Java is required by io.grpc lib grpc-protobuf3.

[dvgica]

Hmm sorry about that, was too hasty. Would it make sense to bump protobuf-java then, and also add the Runtime scope to make it more clear it's not required for compile?

[pjfanning]

I can look at this in more detail later. So far, we have been conservative when upgrading dependencies. Any changes so far have been to lower the number of dependencies with CVEs. We tend to make more people unhappy with significant upgrades. In the end, each connector is pretty small code wise. Users can take our code and adjust it to suit their own needs.

If you can provide a test case where it is proven that there is an issue that can affect most users, it will improve the chances that we will make changes.

[dvgica]

Thanks, I appreciate it. The main trouble we hit is when mixing Connectors libs with up-to-date GCP Java libs on the same classpath. GCP's gRPC dependencies are such a disaster it seems like the only sane way to tackle it is by keeping everything on the latest versions. A test case would be tricky, because certain clients need to be on the class path at specific versions to produce a runtime error.

The particular thing we hit was this (omitted irrelevant parts of the trace):

com.google.api.gax.rpc.ApiException: 
	at com.google.cloud.pubsublite.internal.CheckedApiException.<init>(CheckedApiException.java:51)
	at com.google.cloud.pubsublite.internal.CheckedApiException.<init>(CheckedApiException.java:55)
	at com.google.cloud.pubsublite.internal.ExtractStatus.toCanonical(ExtractStatus.java:53)
	at com.google.cloud.pubsublite.cloudpubsub.PublisherSettings.getAdminClient(PublisherSettings.java:238)
	at com.google.cloud.pubsublite.cloudpubsub.PublisherSettings.instantiate(PublisherSettings.java:260)
	at com.google.cloud.pubsublite.cloudpubsub.Publisher.create(Publisher.java:27)
....
Caused by: java.lang.NoSuchMethodError: 'com.google.longrunning.OperationsClient com.google.longrunning.OperationsClient.create(com.google.longrunning.stub.OperationsStub)'
	at com.google.cloud.pubsublite.v1.AdminServiceClient.<init>(AdminServiceClient.java:188)
	at com.google.cloud.pubsublite.v1.AdminServiceClient.create(AdminServiceClient.java:169)
	at com.google.cloud.pubsublite.cloudpubsub.PublisherSettings.getAdminClient(PublisherSettings.java:230)

This was with "com.google.cloud" % "google-cloud-pubsublite" % "1.12.17" on the classpath.

I did actually wonder about tackling this by shading the GCP/proto/gRPC deps in the Connectors lib. Not something I have a ton of experience with, and I believe it's somewhat of an unsolved problem in Scala, but I did manage to get something going with sbt-shaded. Didn't really have time to test it out yet and was unsure if this approach would be acceptable... in particular I was unsure how far I should go with the shading. Any thoughts on that approach?

[pjfanning]

I was trying to suggest you copy paste the code that you need, inline it and compile it with your favourite jar dependencies. gRPC is in the same ballpark as Hadoop in terms of the number of dependencies it brings and the minefield of managing that when you have other dependencies that bring clashing version requirements.

[dvgica]

Yep, got it. Definitely something we can consider. Was trying to be a good open-source citizen but understand that it's hard to balance the needs of many different users.

[mdedetrich]

I can look at this in more detail later. So far, we have been conservative when upgrading dependencies. Any changes so far have been to lower the number of dependencies with CVEs. We tend to make more people unhappy with significant upgrades. In the end, each connector is pretty small code wise. Users can take our code and adjust it to suit their own needs.

If you can provide a test case where it is proven that there is an issue that can affect most users, it will improve the chances that we will make changes.

Even if it is a valid issue, I still think such a significant change should be marked for pekko connectors 1.1.x, although nothing is stopping us from creating a 1.1.x branch and even a milestone if it helps

[pjfanning]

lgtm