Add python bindings for protobufs #172

chasen-bettinger · 2023-03-28T01:50:30Z

Add command to generate python bindings in Makefile.
Included the generated python bindings that generated from that command in the Makefile.

This closes #171 .

marcelamelara · 2023-03-28T14:44:45Z

Thanks for this PR @chasen-bettinger ! I ran the protoc --python_out command on my local machine, and saw the same v1/0/ directory structure, so I'm sure it's not something on your end... I suspect we might have the same problem when we create the bindings for other languages.

We definitely don't want a hack. I'm thinking we might have to change the directory structure for the protobufs. @TomHennen any ideas?

marcelamelara

Thanks! I'm also realizing a few other obstacles. (1) The __init__.py files are needed at a minimum to make this a proper python package. We probably also want a setup.py file to actually allow installs. (2) The package base name io.in_toto.attestation isn't preserved for the Python bindings right now. I think that's related to our current directory structure (per #172 (comment)).

adityasaky · 2023-03-28T15:15:15Z

Side note: let's use a pyproject.toml file rather than setup.py.

TomHennen

Thanks for doing this!

I think we definitely need the Makefile rule.

As for the generated code, we had checked in the go stubs because it's pretty standard for go code to check that stuff in directly (and it's how go can find the stuff IIUC).

For other languages (like Python) I wonder if it would be preferable to publish a package to PyPI?

Consider this optional. I'll defer to others.

adityasaky · 2023-03-28T17:03:40Z

For other languages (like Python) I wonder if it would be preferable to publish a package to PyPI?

I think we will want to but we have to be mindful of the interplay with https://pypi.org/project/in-toto/. The other alternative is to store generated py files in in-toto/in-toto but I don't like the sound of that...

adityasaky · 2023-03-28T17:04:38Z

Perhaps an in-toto-attestations package in-toto/in-toto uses as a dependency is fine. @lukpueh wdyt?

marcelamelara · 2023-03-28T17:09:40Z

For other languages (like Python) I wonder if it would be preferable to publish a package to PyPI?

I think we will want to but we have to be mindful of the interplay with https://pypi.org/project/in-toto/. The other alternative is to store generated py files in in-toto/in-toto but I don't like the sound of that...

I was thinking about this too. Would it make sense to have an in_toto.attestation sub-package that is pushed to PyPI? I don't know enough about python packaging to know if this would work as intended, but the attestation sub-package does reasonably belong in the in_toto namespace imo.

marcelamelara · 2023-03-28T17:13:44Z

FWIW, the sources for the protobuf python package live in the same repo as the protos. I wonder if we could have a setup similar to them.

Also, to have a consistent package structure for a language bindings in the future, I recommend we move the protos to a separate directory structure: protos/in_toto/attestation/ and protos/in_toto/attestation/predicates. We might want to change the package name of the protos again along with this, but I'm assuming they aren't being imported in too many places right now. WDYT?

adityasaky · 2023-03-28T18:42:19Z

I was thinking about this too. Would it make sense to have an in_toto.attestation sub-package that is pushed to PyPI? I don't know enough about python packaging to know if this would work as intended, but the attestation sub-package does reasonably belong in the in_toto namespace imo.

Note: I'm not necessarily proficient with Python packaging.

My understanding is PyPI doesn't support namespaces like NPM does. To have it be in_toto.attestation, the generated code would need to live in in-toto/in-toto and uploaded with releases of that implementation, I believe. We could just release a separate package built from this repository called in-toto-attestations. Then, in_toto.attestation could import *? Not sure what the benefits of doing that are as folks are more likely to directly use in-toto-attestations anyway...

IMO, we should make in-toto-attestations the home of all models. in-toto/in-toto then uses that and defines user friendly workflows like in-toto-run and in-toto-verify to interact with metadata.

marcelamelara · 2023-03-28T19:03:28Z

My understanding is PyPI doesn't support namespaces like NPM does. To have it be in_toto.attestation, the generated code would need to live in in-toto/in-toto and uploaded with releases of that implementation, I believe.

I feared as much.

IMO, we should make in-toto-attestations the home of all models.

Completely agree. I think there'd be a way to use Python's namespace packages when building and deploying a single in_toto package, but I'm coming around to the idea that it might make more sense to do release the models separately anyway.

python/v1/0/statement_pb2.py

marcelamelara · 2023-04-11T21:25:40Z

@chasen-bettinger We just merged #180. Could you please rebase your branch and regenerate the python bindings given the new directory structure? If you could please update the Makefiles (there's a top-level and one in protos now), that would be really helpful!

- Added Makefile command to generate these bindings - Ran into trouble with the folder path not being preserved. You'll notice that the generated files are in v1/0 instead of the to-be expected v1.0/. That is not intentional, but I'm not sure how to correct this without implementing a hack. Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Additionally, include the python bindings that were created from the exeuction of that Makefile command. Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

chasen-bettinger · 2023-04-12T00:05:10Z

@marcelamelara Thanks for the update. Ready for your re-review!

Makefile

protos/Makefile

python/v1/statement_pb2.py

marcelamelara

Thanks @chasen-bettinger ! I have a few minors changes.

Makefile

python/v1/statement_pb2.py

protos/Makefile

- Add make command for python bindings in primary Makefile - Retain new line in primary Makefile - Adjust Makefile in protos directory such that the python bindings do not use the -I flag because of the intended directory structure and the python target was added to .PHONY. Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

chasen-bettinger · 2023-04-13T20:16:11Z

@marcelamelara Tag, you're it!

marcelamelara

Thanks @chasen-bettinger !! LGTM :) This sets us up really nicely to get the python package released!

marcelamelara · 2023-04-13T20:24:29Z

@adityasaky @TomHennen I figure we can update the documentation separately when this PR is merged and we're done setting up everything else we need to release to PyPI?

adityasaky

Thanks @chasen-bettinger!

marcelamelara · 2023-04-14T20:28:08Z

Will merge after #192 is merged.

TomHennen

Thanks!

marcelamelara requested a review from a team March 28, 2023 14:45

marcelamelara reviewed Mar 28, 2023

View reviewed changes

marcelamelara requested a review from a team March 28, 2023 14:56

TomHennen requested changes Mar 28, 2023

View reviewed changes

adityasaky reviewed Mar 28, 2023

View reviewed changes

python/v1/0/statement_pb2.py Outdated Show resolved Hide resolved

marcelamelara mentioned this pull request Mar 29, 2023

Move protos to their own directory for more consistency between different language bindings #180

Merged

chasen-bettinger force-pushed the 171-python-statement-bindings branch from 09cf039 to 8c60a55 Compare April 11, 2023 23:55

Add command in Makefile to generate python libs

dd2d933

Additionally, include the python bindings that were created from the exeuction of that Makefile command. Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

chasen-bettinger requested a review from marcelamelara April 12, 2023 00:05

adityasaky reviewed Apr 12, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

protos/Makefile Outdated Show resolved Hide resolved

python/v1/statement_pb2.py Outdated Show resolved Hide resolved

marcelamelara requested changes Apr 12, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

python/v1/statement_pb2.py Outdated Show resolved Hide resolved

protos/Makefile Outdated Show resolved Hide resolved

protos/Makefile Outdated Show resolved Hide resolved

chasen-bettinger requested a review from marcelamelara April 13, 2023 20:15

marcelamelara requested review from TomHennen and adityasaky April 13, 2023 20:19

marcelamelara approved these changes Apr 13, 2023

View reviewed changes

adityasaky approved these changes Apr 13, 2023

View reviewed changes

TomHennen approved these changes Apr 20, 2023

View reviewed changes

marcelamelara merged commit 4150fea into in-toto:main Apr 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add python bindings for protobufs #172

Add python bindings for protobufs #172

chasen-bettinger commented Mar 28, 2023 •

edited

Loading

marcelamelara commented Mar 28, 2023

marcelamelara left a comment •

edited

Loading

adityasaky commented Mar 28, 2023

TomHennen left a comment

adityasaky commented Mar 28, 2023

adityasaky commented Mar 28, 2023

marcelamelara commented Mar 28, 2023 •

edited

Loading

marcelamelara commented Mar 28, 2023 •

edited

Loading

adityasaky commented Mar 28, 2023

marcelamelara commented Mar 28, 2023 •

edited

Loading

marcelamelara commented Apr 11, 2023

chasen-bettinger commented Apr 12, 2023

marcelamelara left a comment

chasen-bettinger commented Apr 13, 2023

marcelamelara left a comment

marcelamelara commented Apr 13, 2023

adityasaky left a comment

marcelamelara commented Apr 14, 2023

TomHennen left a comment

Add python bindings for protobufs #172

Add python bindings for protobufs #172

Conversation

chasen-bettinger commented Mar 28, 2023 • edited Loading

marcelamelara commented Mar 28, 2023

marcelamelara left a comment • edited Loading

Choose a reason for hiding this comment

adityasaky commented Mar 28, 2023

TomHennen left a comment

Choose a reason for hiding this comment

adityasaky commented Mar 28, 2023

adityasaky commented Mar 28, 2023

marcelamelara commented Mar 28, 2023 • edited Loading

marcelamelara commented Mar 28, 2023 • edited Loading

adityasaky commented Mar 28, 2023

marcelamelara commented Mar 28, 2023 • edited Loading

marcelamelara commented Apr 11, 2023

chasen-bettinger commented Apr 12, 2023

marcelamelara left a comment

Choose a reason for hiding this comment

chasen-bettinger commented Apr 13, 2023

marcelamelara left a comment

Choose a reason for hiding this comment

marcelamelara commented Apr 13, 2023

adityasaky left a comment

Choose a reason for hiding this comment

marcelamelara commented Apr 14, 2023

TomHennen left a comment

Choose a reason for hiding this comment

chasen-bettinger commented Mar 28, 2023 •

edited

Loading

marcelamelara left a comment •

edited

Loading

marcelamelara commented Mar 28, 2023 •

edited

Loading

marcelamelara commented Mar 28, 2023 •

edited

Loading

marcelamelara commented Mar 28, 2023 •

edited

Loading