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 client proto output makes it difficult to compile new protos for use in clients #6376

Closed
niloc132 opened this issue Nov 14, 2024 · 0 comments · Fixed by #6379
Closed
Labels
bug Something isn't working grpc python-client release blocker A bug/behavior that puts is below the "good enough" threshold to release. triage
Milestone

Comments

@niloc132
Copy link
Member

The pydeephaven package currently provides the generated protoc output in the pydeephaven.proto namespace, despite the proto files having originated in a deephaven/proto/ directory - care was taken to rewrite python references so that we could relocate these, and avoid confusion between deephaven (server side module) and pydeephaven (client side module that uses grpc+proto to communicate).

Unfortunately, this makes it difficult for downstream projects to define new .proto files and compile them to python, and successfully have them reference the types in pydeephaven (for example Ticket in pydeephaven.proto.ticket_pb2), because the actual contents of the .proto (including its directory structure?) is serialized into the generated python files and is expected to match when loaded. The assumption by protoc is that python packaging follows directory structure and no additional nesting/vendoring/renaming would ever happen. In our case, this renaming is necessary, as deephaven has a module which will fail if a JVM is not already started in the current process - so any venv with both pydeephaven and deephaven-core installed cannot be used purely as a client.

protocolbuffers/protobuf#17663
protocolbuffers/protobuf#19212
protocolbuffers/protobuf#1491 esp protocolbuffers/protobuf#1491 (comment)
protocolbuffers/protobuf#7470

Rather than rename every language to use pydeephaven/proto as the directory structure (JS for example is another language that uses directory structure in its output), we're renaming the parent directories of our proto files to be deephaven_core/proto/, as this is language agnostic, and still leaves some space for more than one package. The deephaven python client then will also include this namespace package.

To futureproof any possible deephaven-core protobuf usage, no __init__.py should be added to deephaven_core/ or the nested proto/ directory.

@niloc132 niloc132 added bug Something isn't working grpc triage release blocker A bug/behavior that puts is below the "good enough" threshold to release. python-client labels Nov 14, 2024
@niloc132 niloc132 added this to the 0.37.0 milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working grpc python-client release blocker A bug/behavior that puts is below the "good enough" threshold to release. triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant