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

Add constraint to protocol buffer lib #1090

Merged
merged 6 commits into from
Dec 5, 2024
Merged

Conversation

dexamundsen
Copy link
Contributor

@dexamundsen dexamundsen commented Dec 5, 2024

Issue:

  • starting GCP bom '2.50.0' (current version), the bom also adds protobuf-java libs 4.* version. Previous version did not, and tanagra projects were required to specify the required versions.
  • GCP bom is added in buildsrc plugin, hence any override in sub project of the transitive lib is ignored.
  • minor version upgrades are typically NOT breaking changes, however the GCP bom minor version change lead to a breaking change in the included transitive lib

in protobuf 3.* --> https://www.javadoc.io/doc/com.google.protobuf/protobuf-java/3.25.5/com/google/protobuf/GeneratedMessageV3.ExtendableMessage.html
this includes multiple overloaded declarations of getExtention()

in protocol 4.* --> https://www.javadoc.io/doc/com.google.protobuf/protobuf-java/4.28.3/com/google/protobuf/GeneratedMessage.ExtendableMessage.html
this includes limited overloaded declarations of getExtention(), others are deprecated

--

Fix:

  • Constrain protobuf libs in buildSrc, since the GCP bom is already included in the same buildsrc plugin
  • to add this constraint to the bom, it is to be done via dependency management. Hence move the dependency management plugin to buildSrc as well

--

Testing

tanagra (dexamundsen/indexer) >> tanagra index entity --names=person --indexer-config=cmssynpuf_verily
Indexing completed with 0 failures out of 5 jobs.
Log statements written to: /Users/dexamundsen/.tanagra/logs/tanagra.log
Results report written to: /Users/dexamundsen/github/tanagra/tanagra-indexing.html

@dexamundsen dexamundsen self-assigned this Dec 5, 2024
Copy link
Collaborator

@freemabd freemabd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix.

@dexamundsen dexamundsen merged commit a392b40 into main Dec 5, 2024
8 checks passed
@dexamundsen dexamundsen deleted the dexamundsen/indexer branch December 5, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants