Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Refresh Python module #176

Closed
wants to merge 9 commits into from
Closed

Refresh Python module #176

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
602cf28
Refresh Python module
Dreamsorcerer May 24, 2023
0b3ec5f
Update test_client.py
Dreamsorcerer May 24, 2023
4d0249c
Update test_client.py
Dreamsorcerer May 24, 2023
0803966
Update min versions
Dreamsorcerer Jun 5, 2023
51597aa
Update script instead of README
Dreamsorcerer Jun 5, 2023
325c8a2
Update tests
Dreamsorcerer Jun 5, 2023
43f2556
Merge master
Dreamsorcerer Aug 17, 2023
fa89859
Bump dependency and improve compatibility of script
Dreamsorcerer Aug 17, 2023
ab2d154
Merge master
Dreamsorcerer Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions python/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
pb2 files can be regenerated by running:

```
python3 -m grpc_tools.protoc -I../proto/ --python_out=gubernator/ --pyi_out=gubernator/ --grpc_python_out=gubernator/ --mypy_grpc_out=gubernator/ ../proto/*.proto
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this stuff to a bash script in proto directory? And include a pip install for the tools.

And preferably we shouldn't need post-processing with sed commands. Looks like it's a matter of getting CWD or input paths right to fix that.

Copy link
Author

Choose a reason for hiding this comment

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

And preferably we shouldn't need post-processing with sed commands. Looks like it's a matter of getting CWD or input paths right to fix that.

Literally spent a couple of hours looking at this. In summary, it appears that Google have no interest in supporting modules which would require relative imports. It might be possible to get something to work by changing the proto directory, but I figured that might break things elsewhere. Feel free to have a go yourself though.

Copy link
Author

Choose a reason for hiding this comment

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

protocolbuffers/protobuf#1491

Copy link
Contributor

Choose a reason for hiding this comment

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

As you can probably tell, we don't use this Python functionality anymore. At least, not for Python 3. I'm thinking the entire Python client needs to be reimplemented from protos and disregard any consideration for legacy requirements.

I committed some changes to my branch for a genproto.sh script that already exists but needed some cleaning up. It does generate the code for Go and Python ok. There's some old unit tests in python/tests, but no idea how compatible that is to now current code. It fails, as you might expect.

Copy link
Author

@Dreamsorcerer Dreamsorcerer May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The sed calls are not related to legacy (in fact, it apparently worked on Python 2 without rewriting the imports). It is because Google apparently just dumps all (thousands) of their files in a top-level directory that is on their PYTHONPATH.

For an installable package, we can't expect users to change their PYTHONPATH to point to the package's source code (I know that sound ridiculous, but I literally don't understand why the protobuf maintainers in that thread don't appear to understand that). So, sed rewrites the imports to import from the package (i.e. it could be done either as from gubernator or from .).

To test, run pip install . in your python directory, then (ensuring you're not in the source directory) open python and import gubernator. Without the sed changes you'll get an import error.

I'll go through the other changes next week.

sed -i '' 's/import gubernator_pb2/from gubernator import gubernator_pb2/' gubernator/gubernator_pb2_grpc.py*
sed -i '' 's/import gubernator_pb2/from gubernator import gubernator_pb2/' gubernator/peers_pb2.py*
sed -i '' 's/import peers_pb2/from gubernator import peers_pb2/' gubernator/peers_pb2_grpc.py*
```
29 changes: 8 additions & 21 deletions python/gubernator/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,8 @@
# This code is py3.7 and py2.7 compatible

import gubernator.ratelimit_pb2_grpc as pb_grpc
from datetime import datetime

import time
import grpc

MILLISECOND = 1
SECOND = MILLISECOND * 1000
MINUTE = SECOND * 60


def sleep_until_reset(reset_time):
now = datetime.now()
time.sleep((reset_time - now).seconds)


def V1Client(endpoint='127.0.0.1:9090'):
channel = grpc.insecure_channel(endpoint)
return pb_grpc.RateLimitServiceV1Stub(channel)
Dreamsorcerer marked this conversation as resolved.
Show resolved Hide resolved
from gubernator.gubernator_pb2 import (
Algorithm, Behavior, GetRateLimitsReq, RateLimitReq, Status
)
from gubernator.gubernator_pb2_grpc import V1Stub
from gubernator.peers_pb2_grpc import PeersV1Stub

__all__ = ("Algorithm", "Behavior", "GetRateLimitsReq", "RateLimitReq", "Status",
"PeersV1Stub", "V1Stub")
Loading