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

Python: Consider using betterproto for models? #291

Open
woodruffw opened this issue Oct 18, 2023 · 5 comments
Open

Python: Consider using betterproto for models? #291

woodruffw opened this issue Oct 18, 2023 · 5 comments

Comments

@woodruffw
Copy link
Contributor

Opening this as an idea; I'm unsure if it's a good one.

Context: sigstore-protobuf-specs uses betterproto for its Protobuf codegen, rather than the protobuf package. This has a few API-side advantages, namely cleaner model generation and better mypy/typing integration.

Given that in-toto-attestation and sigstore-protobuf-specs are mutualistic, it might make sense to unify on a single Protobuf base library. I've suggested betterproto for the reasons above, although I could also potentially convinced that protobuf is the better choice 🙂

@woodruffw
Copy link
Contributor Author

Another separate benefit of betterproto here is that it would avoid "laddered" dependencies: turning a Statement object into JSON currently requires the depender to directly depend on protobuf as well, so that it can import MessageToJson.

@joshuagl
Copy link
Contributor

joshuagl commented Dec 5, 2023

Thanks for the suggestion! Better mypy/typing integration alone seems worth the effort.

@woodruffw
Copy link
Contributor Author

Glad to hear it! It looks like in-toto-attestation is still pre-1.0 on PyPI, so this could be done without violating SemVer. But it's still a relatively big API change.

@woodruffw
Copy link
Contributor Author

(As a datapoint: #300 is an example of a typing bug that betterproto would probably have avoided, since those wrapper types wouldn't have been necessary with it.)

@woodruffw
Copy link
Contributor Author

Looking into this today.

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 a pull request may close this issue.

2 participants