Skip to content
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

Generate native image reflect-config.json for com.google.firestore.bundle #2011

Open
mpeddada1 opened this issue Sep 19, 2023 · 5 comments
Open
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@mpeddada1
Copy link
Contributor

We generate configurations for com.google.firestore.v1 classes but not com.google.firestore.bundle classes. These needed to be manually added in googleapis/java-firestore#1419.

@mpeddada1 mpeddada1 added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Sep 19, 2023
@lqiu96 lqiu96 added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Sep 20, 2023
@blakeli0
Copy link
Collaborator

blakeli0 commented Sep 20, 2023

Do we know how the com.google.firestore.bundle classes are added to java-firestore repo? I don't see them being passed into the generator in the Bazel build file.

@mpeddada1
Copy link
Contributor Author

The bazel rule for assembling this code is located under the google/firestore/bundle directory. Originally, it looks like this package was manually generated (googleapis/java-firestore#271) in the repo and then the bazel rules were migrated over to third_party?

@blakeli0
Copy link
Collaborator

I see, this is a proto only module that does not involve the generator, it is generated using protoc plugin directly. If that's the case, I don't think we have a way to generate it through generator.
Is this the only occurrence of this issue? Do you see it happening in other handwritten libraries?

@mpeddada1
Copy link
Contributor Author

mpeddada1 commented Sep 27, 2023

Hm I could be wrong but these classes seem to get generator updates through Owlbot (example, googleapis/java-firestore#1304) which make me think that they aren't fully disconnected?

Is this the only occurrence of this issue? Do you see it happening in other handwritten libraries?

From what we've observed so far, Firestore is the only one that has this issue. From the native image perspective, Spanner needed manually added configs but for different reason as there were explicitly using + reflectively calling some com.google.rpc classes (googleapis/java-spanner#2617)

@blakeli0 blakeli0 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Dec 19, 2023
@blakeli0
Copy link
Collaborator

Hm I could be wrong but these classes seem to get generator updates through Owlbot (example, googleapis/java-firestore#1304) which make me think that they aren't fully disconnected?

Yes they are generated into googleapis-gen through the Bazel configurations , hence we can configure owlbot to copy the code from googleapis-gen. But the Bazel configurations only include firestore_bundle_java_proto and firestore_bundle_proto, which do not go through the gapic-generator-java code, compare to a firestore v1 Bazel configuration, that includes a firestore_java_gapic dependency.

This issue suffers from the same root cause as I mentioned in #2048, if we do prioritize these issues, I would suggest to fix #2048 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants