Protocol buffer compiler for Scala. Consider sponsoring ScalaPB: https://github.com/sponsors/thesamet
Hello guys, I've been trying to make Scalapb (scala + validation) work together with buf in this example repo here.
I am running MacOS and the latest versions of the plugins (protoc-gen-scala and protoc-gen-scalapb-validate) are on my PATH.
Everything works fine until I want the validation to happen during the construction by setting the extension options in here which results in compilation failure like this one here.
Any ideas how to fix that would be much appreciated.
@thesamet I've added the validate/validate.proto manually to the repository and using plain protoc without buf has the same result.
It's also possible that I am not doing it right, however without the validation options it generates correct output.
@karadzhov , I was able to reproduce the issue. You are using the native scalapb plugin which is built with Scala Native. The Java code that runs to parse the input protos relies on Java reflection. With Scala Native, all the reflective calls need to be known at compile time. To accomplish this in ScalaPB we rely on our test coverage to exercise all the code paths that could possibly trigger reflection. I added a new test for extensions of scalapb options in the same manner that scalapb-validate does: scalapb/ScalaPB@392a164 - this did the trick for the example you sent.
Can you try the snapshot release of protoc-gen-scala from the artifact section here: https://github.com/scalapb/ScalaPB/actions/runs/1410779035 and see if it fully solves your use case? From some strange reason, there would be another zip within the zip that you'll download from there.
@thesamet thank you very much for your prompt response and solution. I've tried the snapshot artifact and can confirm that the compilation using protoc directly doesn't fail with the exception anymore.
I've tested the compilation using buf with different configurations and found out that when validate_at_construction and insert_validator_instance are both set to true there are warnings like this one "Warning: Duplicate generated file name "example/common/certificate/v1/CertificateSigningRequest.scala". Buf will continue without error here and drop the second occurrence of this file, but please raise an issue with the maintainer of the plugin." and the output is wrong.
Although this problem does not occur while using protoc directly I still wonder whether you would be able to solve the problem with buf by modifying the generator somehow.
Hi @karadzhov , this is a bug in buf that does not have a workaround on the ScalaPB side. The ScalaPB validator plugin uses an insertion feature of the protoc plugin protocol, so the validator plugin can insert validation code into the code generated by the core ScalaPB plugin. buf doesn't seem to support this, and I can file a bug with that project, so we can track this.
How do you invoke scalapb-validate through buf? Can you help me reproduce this part?
@thesamet Thank you!
Please let me ask you about the final piece of my puzzle so far which is the gRPC.
I really would like to use fs2-grpc mainly because of the cats integration so that i don't need to wrap manually the futures everywhere. (btw is it intentional that the documentation has a link to a fork which is a bit behind from the upstream).
I assume it won't be as straightforward as scalapb-validate because I wasn't able to find any protoc-gen plugin built from the codegen. What would you advise?
@thesamet I am looking forward to the next buf release.
Meanwhile I've tried to find the fs2-grpc compiler plugin executable but without much success.
Scalapb-validate's is published alongside the jars however fs2-grpc's is not there. In case I am looking at the wrong artifact, would you please point me to the correct one. Thank you :)
protocVersion, but possibly there are uses of it that don't cause this error (if ScalaPB isn't invoked for example). For the specific use case you have, I suggest to use recent protoc and protobuf-java and use shading to avoid binary compatibility issues with Spark: https://scalapb.github.io/docs/sparksql#setting-up-your-project.
@thesamet I though it must be there already because that particular version was built after your PR got merged. I will try to reach out to the maintainers.
Regarding the Scalapb itself it would be much simpler for development if there was a Scalapb organization in the buf registry that publishes the proto files which later could be used like this instead of copying them manually.
@ngbinh yes you can find a simple example over here
However, from my point of view, the PR should be done the scalapb compiler. The NPE happens in Person.messageReads (which is generated by the scalapb compiler).
Or can it be handled anywhere else ? In the ProtoSQL ?
Maybe I found a way to fix it.
diff --git a/sparksql-scalapb/src/main/scala/scalapb/spark/FromCatalystHelpers.scala b/sparksql-scalapb/src/main/scala/scalapb/spark/FromCatalystHelpers.scala
--- a/sparksql-scalapb/src/main/scala/scalapb/spark/FromCatalystHelpers.scala (revision 28d88a90150c9242ee9e3a57ff3b86da6f233ae9)
+++ b/sparksql-scalapb/src/main/scala/scalapb/spark/FromCatalystHelpers.scala (date 1636475671326)
@@ -69,8 +69,13 @@
input: Expression
): Expression = {
if (fd.isRepeated && !fd.isMapField) {
+ val nonNullInput = If(
+ IsNull(input),
+ input,
+ Literal.fromObject(Array.empty[String])
+ )
val objs = MapObjects(
- (input: Expression) => singleFieldValueFromCatalyst(cmp, fd, input),
+ (input: Expression) => singleFieldValueFromCatalyst(cmp, fd, nonNullInput),
input,
protoSql.singularDataType(fd)
)This does not work "as-is". I am not very fluent with Catalyst ^^'
But this should be a good start.
singularDataType of fd.
@thesamet: This seems to work.
diff --git a/sparksql-scalapb/src/main/scala/scalapb/spark/JavaHelpers.scala b/sparksql-scalapb/src/main/scala/scalapb/spark/JavaHelpers.scala
--- a/sparksql-scalapb/src/main/scala/scalapb/spark/JavaHelpers.scala (revision 07217938678b93309fb2a148f181e673fbfef21d)
+++ b/sparksql-scalapb/src/main/scala/scalapb/spark/JavaHelpers.scala (date 1636493180889)
@@ -82,7 +82,10 @@
def mkMap(cmp: GeneratedMessageCompanion[_], args: ArrayData): Map[FieldDescriptor, PValue] = {
cmp.scalaDescriptor.fields
.zip(args.array)
- .filterNot(_._2 == PEmpty)
+ .filter {
+ case (_, null) | (_, PEmpty) => false
+ case _ => true
+ }
.toMap
.asInstanceOf[Map[FieldDescriptor, PValue]]
}Too late to open the PR, I will do it tomorrow. Are you OK with the pattern matching or do you prefer another code construction.
Thanks for the advice. Hope it will solve my problem (I will test it on my production code tomorrow).
I need to backport this PR to scalapb-sparksql v0.10.4...
mkMap does not seems to exist in this version...
I don't know if you are interested to merge it too? In this case, you have to create a branch for the 0.10.5 release on which I can open a PR.
It happens with a highly nested message, but on a root field which is a repeated nested message.
message Auction {
Request request...
reapeated Response responses...
...
}If responses is null it fails. But it seems to handle the other null-repeated fields properly.
addresses field of the Person message. So, I don't know what can cause this error...
spark.implicits._ and the protoSql.implicits._