alexshtin
commented
4 years ago Thanks for reporting this, @firdaus, at the very right moment!
We will fix 2nd item soon because we gonna prefix all enum values with type prefix as recommended here.
For the 1st one, getting rid of recursive relationship for Failure will break semantic. Original idea was to use something like FailureChain with repeated failures inside. But this will add complexity to both SDKs and server. I believe moving forward with your own fork of python-betterproto is a better option for now. Also I hope this bug will be fixed there sooner or later.
This isn't really an issue/feature request.
It's more of an observation on issues I had when integrating the temporal-proto into the Python SDK using python-betterproto.
The default GRPC libraries for Python leave a lot to be desired. So I was looking into using python-betterproto which gives us a much better Python experience for GRPC.
However, I ran into a few issues that I would like to highlight here. The issues are actually not the fault of temporal-proto at all. It's mainly due to the default behavior of python-betterproto. I looked into the code for python-betterpro and I think it's easy enough to fork and modify it for my needs. So there is no need for any action on Temporal's side.
Having said that, I am simply highlighting this before the window for backwards incompatible changes closes for the Temporal team's possible consideration:
1.) Recursive relationship: Failure
python-betterproto attempts to assign a default value to each field. This leads to the stack space being exhausted when the a Failure object is initialized because it attempts to build an object graph which never ends. I think this is because python-betterproto doesn't support HasField/ClearField functions like other protobuf implementations.
I already fixed this issue in my local copy of python-betterproto by simple not initializing sub-message fields when instantiating messages.
2.) "None" in enum QueryRejectCondition is a keyword in Python
I believe this is also quite easy to fix:
https://github.com/danielgtaylor/python-betterproto/blob/f8203977513d5d7c821cbac63e6fa764d301c2e8/betterproto/casing.py
I will need to fork python-betterproto whether or not any changes are made in temporal-proto for the two above issues --- in particular the generated RPC stub methods use parameters instead of request objects --- therefore it really is not much trouble at all for me to make accommodations for the two above issues.