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

Update protobuf, OSM parser and Google cloud tools #6342

Merged

Conversation

leonardehrenfried
Copy link
Member

@leonardehrenfried leonardehrenfried commented Dec 17, 2024

Summary

It turned out that we never actually set the version of the protbuf java library and compiler and relied on it being set by the OSM and Google Cloud libraries.

Starting with version 26.51.0 of Google Cloud it introduced a new version of protobuf that was incompatible with the one used by the OSM library and the tests started failing.

For this reason I updated the OSM library and used the newest version of protobuf. This means that we can now also update Google Cloud.

Since the new protbuf Java runtime has change a bit, some protobuf-related classes had to be adjusted.

Unit tests

The unit tests that parse real OSM files prevented something bad from happening.

Bumping the serialization version id

I don't think it's required since protbuf is only used for imports, but I can set the tag if requested.

@leonardehrenfried leonardehrenfried requested a review from a team as a code owner December 17, 2024 22:02
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.80%. Comparing base (b0b8661) to head (f53245f).
Report is 17 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...pplanner/updater/trip/MqttGtfsRealtimeUpdater.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #6342   +/-   ##
==========================================
  Coverage      69.80%   69.80%           
- Complexity     17941    17942    +1     
==========================================
  Files           2046     2046           
  Lines          76672    76671    -1     
  Branches        7830     7830           
==========================================
+ Hits           53518    53521    +3     
+ Misses         20412    20408    -4     
  Partials        2742     2742           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

optionsome
optionsome previously approved these changes Dec 31, 2024
pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
optionsome
optionsome previously approved these changes Jan 3, 2025
@t2gran t2gran added this to the 2.7 (next release) milestone Jan 9, 2025
t2gran
t2gran previously approved these changes Jan 9, 2025
Copy link
Member

@t2gran t2gran left a comment

Choose a reason for hiding this comment

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

You can resolve the conflicts and merge without me re-approve.

optionsome
optionsome previously approved these changes Jan 9, 2025
Copy link
Member

@optionsome optionsome left a comment

Choose a reason for hiding this comment

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

Same as with @t2gran

@leonardehrenfried leonardehrenfried dismissed stale reviews from optionsome and t2gran via 2bf1fde January 9, 2025 15:30
@leonardehrenfried leonardehrenfried merged commit 0b8b82b into opentripplanner:dev-2.x Jan 9, 2025
5 checks passed
@leonardehrenfried leonardehrenfried deleted the upgrade-protobuf branch January 9, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants