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

chore(cd): fix protobuf import issue #204

Merged
merged 7 commits into from
May 30, 2023
Merged

Conversation

joshua-goldstein
Copy link
Contributor

@joshua-goldstein joshua-goldstein commented May 29, 2023

Problem

Currently, calling import pydgraph will complain because of module import path issues. E.g.,

>>> import pydgraph
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/manas/.local/lib/python3.10/site-packages/pydgraph/__init__.py", line 17, in <module>
    from pydgraph.client_stub import *
  File "/home/manas/.local/lib/python3.10/site-packages/pydgraph/client_stub.py", line 20, in <module>
    from pydgraph.proto import api_pb2_grpc as api_grpc
  File "/home/manas/.local/lib/python3.10/site-packages/pydgraph/proto/api_pb2_grpc.py", line 5, in <module>
    import api_pb2 as api__pb2

This is not a new problem with protobufs. See e.g. [1] [2] [3] [4][5]. Essentially, the issue has to do with the generated protobuf files, which make an import which is not by default discoverable. One trick was to use a relative import path, but this is not a great solution and may be causing issues.

Solution

Use __init__.py in the protos package to add the parent directory to the Python path, so that the package will be discovered. This means we no longer have to modify the generated protobuf files.

Joshua Goldstein added 2 commits May 29, 2023 23:56
mangalaman93
mangalaman93 previously approved these changes May 29, 2023
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool, good work on figuring this out. We should do a rc release on Pypi before the actual release and make sure pull and import works as expected.

@MichelDiz
Copy link
Contributor

I had this problem and had to downgrade.

@mangalaman93
Copy link
Contributor

I had this problem and had to downgrade.

What do you mean? I assumed that the import was working for the new release

@MichelDiz
Copy link
Contributor

I had this problem and had to downgrade.

What do you mean? I assumed that the import was working for the new release

Yeah, for some reason it happened to me. I was creating a new cron job(like 1 an half day ago) to delete old data in the telemetry code.
I use Python for that. I got the same error for api__pb2. And I was confused. So I downgraded and everything worked fine again.

@joshua-goldstein joshua-goldstein merged commit 704313c into master May 30, 2023
3 checks passed
@joshua-goldstein joshua-goldstein deleted the joshua/protos branch May 30, 2023 10:02
@joshua-goldstein
Copy link
Contributor Author

joshua-goldstein commented May 30, 2023

@MichelDiz When you have a moment could you try to pull pip install pydgraph==23.0.1rc1? This PR should resolve these issues. If yes, please approve #205 .

joshua-goldstein added a commit that referenced this pull request May 30, 2023
This fixes protobuf import issues.  For more info see #204 .
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.

None yet

4 participants