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 types.proto & types_pb2.py for latest tendermint (0.15) #9

Closed
sbellem opened this issue Jan 16, 2018 · 12 comments
Closed

Update types.proto & types_pb2.py for latest tendermint (0.15) #9

sbellem opened this issue Jan 16, 2018 · 12 comments

Comments

@sbellem
Copy link
Contributor

sbellem commented Jan 16, 2018

The go abci types.proto has changed and the current py-abci does not work with the Tendermint 0.15.

Will add more details and steps to reproduce the problem soon.

Also related: bigchaindb/bigchaindb#2005 (comment)

In the meantime, this issue seems related: tendermint/abci#179

Related: code types (enum CodeType) were removed: tendermint/abci@42a8e32#diff-f325a92d451852ea8923aaa3a50731be

@sbellem sbellem changed the title Update types.proto && types_pb2.py for latest tendermint (0.15) Update types.proto & types_pb2.py for latest tendermint (0.15) Jan 16, 2018
@srmo
Copy link

srmo commented Jan 16, 2018

Subscribing here to see if there's any progress on your side. We have trouble porting jABCI to 0.15+ tendermint because of those new gogoproto types.
jTendermint/jabci#34

@davebryson
Copy link
Owner

So if I'm understanding correctly, this is a fundamental problem with the latest types.proto file in tendermint?

@sbellem
Copy link
Contributor Author

sbellem commented Jan 16, 2018

So if I'm understanding correctly, this is a fundamental problem with the latest types.proto file in tendermint?

Well, since I am not very knowledgeable with protobuf I wish to be careful to make conclusions, but it most certainly complicates things.

I am currently testing the simple approach of removing the annotations, and will report as soon as possible.

@davebryson
Copy link
Owner

davebryson commented Jan 16, 2018

At first glance, it looks like you need to add gogo protobuf to your build chain to properly compile it for Python bindings - but this is a very quick assessment. Here's an example:

https://github.com/bblfsh/client-python/blob/master/setup.py#L86

@sbellem
Copy link
Contributor Author

sbellem commented Jan 16, 2018

Oh, interesting. I have tried using gogoprotobuf, following https://github.com/gogo/protobuf#more-speed-and-more-generated-code but ran into the import problem, reported in cockroachdb/cockroach#393:

FYI, the python-specific issue here is that protoc's python support assumes that the proto directory structure and python package structure are the same: importing github.com/gogo/protobuf/gogoproto/gogo.proto in a proto file translates to "import github.com.gogo.protobuf.gogoproto.gogo_pb2" in python. Using "github" as a top-level python package is basically a non-starter. Even if this were fixed and the top-level package remapped to "gogoproto", someone would need to own the "gogoproto" python package; there's no clean way for cockroach-python to distribute its own vendored version of gogoproto.

I could try the approach used in the link you provided.

In the meantime, the approach of removing the annotation yields positive results, but as pointed out by @srmo it does not mean that there cannot be unnoticed negative side effects.

@srmo
Copy link

srmo commented Jan 16, 2018

I'm currently trying out a more streamlined rebuild with ideas from the links you gave, @sbellem.
Looks promising so far.
What I'm doing is having the gogo.proto from the gogo repo next to the tendermint types proto and do some java options. I'm using the protoc maven plugin and say that it has to include stdTypes (the ones from google) as they are imported from gogo.proto.
This leads to generated files for tendermint.abci and gogoproto which seems to work. Currently still in testing but this looks good. (with the minor caveat that the CodeType type has vanished, nothing a constant class wont fix ;) )

@sbellem
Copy link
Contributor Author

sbellem commented Jan 16, 2018

@davebryson just made a PR to show the naive approach of removing the annotations. You can consult the description of the PR for additional details.

I will resume work on this issue tomorrow. I am ok with helping resolving the issue, so please let me know if you prefer to tackle it yourself, or how I can help.

@davebryson
Copy link
Owner

@sbellem Yes unfortunately I'm currently knee deep in Go programming on Ethereum. So any help you can provide is greatly appreciated!

@sbellem
Copy link
Contributor Author

sbellem commented Jan 18, 2018

@davebryson ah ok, that's fine! Thanks for letting me know!

So I'll take care of the issue then. Will keep you informed via this issue and PR #11 as progress is made..

@sbellem
Copy link
Contributor Author

sbellem commented Jan 23, 2018

At first glance, it looks like you need to add gogo protobuf to your build chain to properly compile it for Python bindings - but this is a very quick assessment. Here's an example:

https://github.com/bblfsh/client-python/blob/master/setup.py#L86

@davebryson see PR #14 😄

@davebryson
Copy link
Owner

Should be resolved with the latest merges thanks to @sbellem. Leaving open for now until we test more.

@davebryson
Copy link
Owner

Closing for now. See abci-0.10.0 branch

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

No branches or pull requests

3 participants