deephaven / deephaven-core

Deephaven Community Core
Other
257 stars 80 forks source link

Python client proto output makes it difficult to compile new protos for use in clients #6376

Closed niloc132 closed 1 day ago

niloc132 commented 1 week ago

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.

https://github.com/protocolbuffers/protobuf/issues/17663 https://github.com/protocolbuffers/protobuf/issues/19212 https://github.com/protocolbuffers/protobuf/issues/1491 esp https://github.com/protocolbuffers/protobuf/issues/1491#issuecomment-1650071473 https://github.com/protocolbuffers/protobuf/pull/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.