-
Notifications
You must be signed in to change notification settings - Fork 130
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
Tsk 757 add support for tuple in payload #763
Conversation
We cannot test tuple as the return value is listvalue, so i only kept the list
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -183,6 +183,7 @@ def test_not_jsonable_payload(): | |||
{"frozenset": frozenset([1, 2])}, | |||
{"set": {1, 2}}, | |||
{"uuid": uuid.uuid4()}, | |||
{"listvalue": [1, 1]}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be tested somewhere else
This test is about not jsonable types in payload, however lists and tuples are jsonable
e.g.
>>> import json
>>> json.dumps({"a": (1,2)})
'{"a": [1, 2]}'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we maybe add it to the random payload generator in fixtures?
why do we need this? |
|
* Add support for tuple in payload for grpc * chore: delete health_check_pb2_grpc.py * chore: Modify payload test case to test tuple datatype * chore: Removed test case for tuple We cannot test tuple as the return value is listvalue, so i only kept the list * chore: Remove excess prints in info endpoint * chore: Added tuple in random payload generator * feat: convert the tuple payload of local mode to list for congruence
All Submissions:
dev
branch. Did you create your branch fromdev
?New Feature Submissions:
pre-commit
withpip3 install pre-commit
and set up hooks withpre-commit install
?Changes to Core Features: