MobilityData / gtfs-realtime-bindings

Language bindings generated from the GTFS Realtime protocol buffer spec for popular languages.
Apache License 2.0
370 stars 127 forks source link

Pin the Python protobuf dependency (#71) #91

Closed jamespfennell closed 1 year ago

jamespfennell commented 2 years ago

The Python protobuf library was updated tonight in a backwards incompatible way, and the Python bindings are no longer working for fresh builds. I tried to upgrade the bindings but couldn't get the tests to pass. As a workaround, this commit just pins the version of the Python protobuf package to something less than 4.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

atvaccaro commented 1 year ago

Hey, has there been any discussion of this issue/PR? We've hit the same problem since poetry will gladly update protobuf since it's not pinned in this repo.

jameslinjl commented 1 year ago

@atvaccaro i'm not aware of any further discussion here, but i think this PR makes sense to merge + publish as a stopgap (cc @isabelle-dr). i have this on my radar to dig further into, but won't get to it right away

isabelle-dr commented 1 year ago

Sounds good @jameslinjl. Just one thing:

publish as a stopgap

What does it mean? šŸ™ˆ

jamespfennell commented 1 year ago

A longer term solution is to upgrade the Python dependencies to their most recent version, however I tried this and the unit tests donā€™t pass so itā€™s more work than this PR. This PR is a ā€œstopgapā€ in the sense that it just deals with the problem in the short term until someone gets around to upgrading the dependencies.

jameslinjl commented 1 year ago

@jamespfennell @atvaccaro I'm actually having some trouble reproducing this issue (caveat: I'm not super familiar with the Python ecosystem, so I could totally be doing something wrong here). I needed to resolve an issue on the unit test where rb was necessary when calling open (since the .pb file is binary data), but otherwise the tests continue to pass, even when I do a clean install with protobuf-4.21.10. Is there some sort of step that I'm missing here?

jamespfennell commented 1 year ago

@jameslinjl if you got the tests to pass with the most recent protobuf release that would be awesome! I think it would be preferable to the current PR.

tannert commented 1 year ago

Any update on this? I just naively installed gtfs-realtime-bindings using pip and got an unusable package.

jameslinjl commented 1 year ago

Hey @tannert ! master should have the required fixes for this already, but I don't believe the latest changes have been published yet. I believe @bdferris-v2 already granted @isabelle-dr the permission to be able to publish the new package version, so I think we just need for her to give that a shot!

jameslinjl commented 1 year ago

hey @tannert and @jamespfennell ! i just published the python bindings 1.0.0, and it seems to be working for me when I test it on my local machine pulling from PyPI. Would one (or both) of you be able to help validate from your side to make sure 1.0.0 looks good?

tannert commented 1 year ago

@jameslinjl just tested 1.0.0 and it looks good! Thanks!

jameslinjl commented 1 year ago

@isabelle-dr could you close this PR since it is no longer relevant / needed?