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

Improve build process of grpc bindings #369

Merged

Conversation

ErikDeSmedt
Copy link
Collaborator

@ErikDeSmedt ErikDeSmedt commented Feb 28, 2024

We are generating/compiling python bindings from .proto-files.

Before this commit the generated protobuf-files did not understand they
were a part of the glclient-module.

This resulted in broken imports which we fixed using sed in our
Makefile.

This commit improves the configuration and voids the need for hacks in
our Makefile.

For more info: See protocolbuffers/protobuf#4176

@ErikDeSmedt ErikDeSmedt force-pushed the better-building-of-grpc-bindings branch from ba49379 to 8f9ea37 Compare February 28, 2024 14:19
@ErikDeSmedt ErikDeSmedt marked this pull request as draft February 28, 2024 14:23
@ErikDeSmedt ErikDeSmedt force-pushed the better-building-of-grpc-bindings branch from 8f9ea37 to 9a8b274 Compare February 28, 2024 22:27
@cdecker
Copy link
Collaborator

cdecker commented Feb 29, 2024

Following up on our discussion re Google path semantics, I took a look at the generated _grpc.py files, and was surprised the URI was unchanged, instead its just the prefix for the serialization and deserialization was changed. This makes my concerns from yesterday invalid ^^

Today I learned 👍

@ErikDeSmedt ErikDeSmedt marked this pull request as ready for review March 1, 2024 12:31
We are generating/compiling python bindings from `.proto`-files.

Before this commit the generated protobuf-files did not understand they
were a part of the `glclient`-module.

This resulted in broken impors which se fixed using `sed` in our
Makefile.

This commit improves the configuration and voids the need for hacks in
our `Makefile`.

For more info: See protocolbuffers/protobuf#4176
@cdecker cdecker force-pushed the better-building-of-grpc-bindings branch from 82599a9 to 7553358 Compare March 1, 2024 12:52
@cdecker
Copy link
Collaborator

cdecker commented Mar 1, 2024

Rebased to remove that merge commit, merging asap

@cdecker cdecker enabled auto-merge (rebase) March 1, 2024 12:52
@cdecker cdecker merged commit a1264c1 into Blockstream:main Mar 1, 2024
16 checks passed
ErikDeSmedt added a commit to ErikDeSmedt/greenlight that referenced this pull request Mar 4, 2024
We use pdoc to generate a reference of the python `glclient`-library.
We suffered some issues related to missing type-hints. The work-around
was to remove the type-hints before generating the docs.

I've found 3 causes of warnings.

1. Missing export of `SignerHandle` (See Blockstream#368)
2. Poorly defined imports in auto-generated GRPC-code (See
   Blockstream#369)
3. pdoc failing to import type from type-stubs
   (See mitmproxy/pdoc#671)

All of these 3 issues have been addressed so we don't need the hack
anymore.
cdecker pushed a commit to ErikDeSmedt/greenlight that referenced this pull request Jun 6, 2024
We use pdoc to generate a reference of the python `glclient`-library.
We suffered some issues related to missing type-hints. The work-around
was to remove the type-hints before generating the docs.

I've found 3 causes of warnings.

1. Missing export of `SignerHandle` (See Blockstream#368)
2. Poorly defined imports in auto-generated GRPC-code (See
   Blockstream#369)
3. pdoc failing to import type from type-stubs
   (See mitmproxy/pdoc#671)

All of these 3 issues have been addressed so we don't need the hack
anymore.
cdecker pushed a commit that referenced this pull request Jun 7, 2024
We use pdoc to generate a reference of the python `glclient`-library.
We suffered some issues related to missing type-hints. The work-around
was to remove the type-hints before generating the docs.

I've found 3 causes of warnings.

1. Missing export of `SignerHandle` (See #368)
2. Poorly defined imports in auto-generated GRPC-code (See
   #369)
3. pdoc failing to import type from type-stubs
   (See mitmproxy/pdoc#671)

All of these 3 issues have been addressed so we don't need the hack
anymore.
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 this pull request may close these issues.

2 participants