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

Refactor: remove betterproto dep #1049

Open
woodruffw opened this issue Jun 20, 2024 · 5 comments
Open

Refactor: remove betterproto dep #1049

woodruffw opened this issue Jun 20, 2024 · 5 comments
Labels
enhancement New feature or request refactoring Refactoring tasks.

Comments

@woodruffw
Copy link
Member

Our dependency on betterproto is a source of a few issues:

  1. We currently depend on the 2.x series, which is only in beta (currently 2.0.0b6). This is both non-ideal and is incompatible with some Python installers, most notably uv: https://github.com/astral-sh/uv/blob/main/PIP_COMPATIBILITY.md#pre-release-compatibility
  2. betterproto's handling of the Struct WKT is partially broken. I've tried to fix it, but round-tripping a JSON serialized message through a Struct is still impossible, which causes all kinds of problems for the (mostly in-toto) definitions that use Struct
  3. Finally, the models and their APIs themselves aren't particularly Pythonic -- this is more of an issue with protobuf itself than betterproto, but results in development friction and impedance mismatches elsewhere that we have to paper over

As such, we should probably remove our dependency on betterproto in the medium term. There are a few blockers to doing this:

  1. The Bundle, etc. models all come from protobuf-specs, which is currently built on top of betterproto for the Python bindings.
  2. There are (probably) a few places we leak references to betterproto-generated models in Sigstore's public APIs

(1) is probably addressable either by re-modeling the relevant parts of protobuf-specs in pydantic, or by regenerating the Python bindings on top of the JSON Schema for protobuf-specs. (2) may require a major reversion.

CC @DarkaMaul since he noticed this 🙂

@woodruffw woodruffw added enhancement New feature or request refactoring Refactoring tasks. labels Jun 20, 2024
@woodruffw
Copy link
Member Author

Not immediately useful, but saving this here so I don't forget it: https://github.com/criccomini/proto-schema-parser

@mgorny
Copy link

mgorny commented Sep 27, 2024

I wanted to report that betterproto 2.0.0b7 was released in August (and that sigstore fails tests against it), but given this issue, is there a point?

@woodruffw
Copy link
Member Author

woodruffw commented Sep 27, 2024

That is indeed good to know! We're still planning on dropping our dependency on it, but we should test against the latest in the mean time. I'll start a PR for that.

Edit: It's technically a transitive dep via sigstore-protobuf-specs, so I'll pursue there.

@mgorny
Copy link

mgorny commented Sep 27, 2024

Thanks! It would really be appreciated, given that b6 is partially broken with modern pydantic (though I don't think it affects sigstore, but its reproduced with its own test suite).

@woodruffw
Copy link
Member Author

sigstore/protobuf-specs#404 has the bump, thanks again! I'll get a version of that package cut and then bump here once it's ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactoring Refactoring tasks.
Projects
None yet
Development

No branches or pull requests

2 participants