You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2021/12/12 21:14:58 UTC
[GitHub] [thrift] kainjow opened a new pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
kainjow opened a new pull request #2489:
URL: https://github.com/apache/thrift/pull/2489
Currently in Python, the generated enum classes for enum thrift objects, are not being used in deserialization (or serialization). This means that the string representations of enums are never used. Moving one step further than the patch contained in THRIFT-2059, this creates python IntEnum classes and retrieves the string representation on deserialization while storing the integer representation on serialization.
[Recreated original PR from #1788 by @constd]
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [ ] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
- [ ] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [ ] Did you squash your changes to a single commit? (not required, but preferred)
- [ ] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] kainjow commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
kainjow commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1059679448
Looks like tests are passing, but some failures with js and erl.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] kainjow commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
kainjow commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1059811766
You could consider turning off Merge commits on the repo and only enable Squash.
On Sat, Mar 5, 2022, at 9:20 AM, Jens Geyer wrote:
>
>
> Nice. Didn't notice that it was 1000 commits
>
>
> —
> Reply to this email directly, view it on GitHub <https://github.com/apache/thrift/pull/2489#issuecomment-1059800069>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACCFOYL35JZ5YR7O3GKEZLU6OJXDANCNFSM5J4WASJA>.
> Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
> You are receiving this because you authored the thread.Message ID: ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
fishy commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1021641037
I'm pretty sure these 2 travis failures do not show before this pr:
* https://app.travis-ci.com/github/apache/thrift/jobs/556647214
* https://app.travis-ci.com/github/apache/thrift/jobs/556647215
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G merged pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
Jens-G merged pull request #2489:
URL: https://github.com/apache/thrift/pull/2489
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
fishy commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1021495638
I do not have enough experience messing with the python compiler 😅
We do have plan to drop support for python 2, or python3 before 3.4 (as they are all EOL now), so if we do this after that we don't really have to do any special handling and can just support it blindly. but doing it now means that we still need to handle the case of <3.4 that `IntEnum` is not available?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G edited a comment on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1059800069
Nice. Didn't notice that it was 1000 commits :-(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1059852029
You could consider squashing as well, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] kainjow commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
kainjow commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1019415615
@fishy @alisaifee @Jens-G
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] kainjow commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
kainjow commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1019515339
A few failures, but seems like flaky tests. I spotted this in 2 runs:
```
Testing script: /usr/bin/python /thrift/src/test/py/FastbinaryTest.py
----
Traceback (most recent call last):
File "/thrift/src/test/py/FastbinaryTest.py", line 42, in <module>
from DebugProtoTest import Srv
ImportError: No module named DebugProtoTest
*** FAILED ***
LIBDIR: /thrift/src/lib/py/build/lib.linux-x86_64-2.7
PY_GEN: gen-py-enum
SCRIPT: FastbinaryTest.py
Traceback (most recent call last):
File "./RunClientServer.py", line 323, in <module>
sys.exit(main())
File "./RunClientServer.py", line 302, in main
runScriptTest(options.libdir, options.gen_base, genpydir, script)
File "./RunClientServer.py", line 103, in runScriptTest
raise Exception("Script subprocess failed, retcode=%d, args: %s" % (ret, ' '.join(script_args)))
Exception: Script subprocess failed, retcode=1, args: /usr/bin/python /thrift/src/test/py/FastbinaryTest.py
[0;31mFAIL[m: RunClientServer.py
[0;31m==================[m
[0;31m1 of 1 test failed[m
[0;31m==================[m
Makefile:532: recipe for target 'check-TESTS' failed
make[4]: *** [check-TESTS] Error 1
make[4]: Leaving directory '/thrift/src/test/py'
Makefile:658: recipe for target 'check-am' failed
make[3]: *** [check-am] Error 2
make[3]: Leaving directory '/thrift/src/test/py'
Makefile:661: recipe for target 'check' failed
make[2]: *** [check] Error 2
make[2]: Leaving directory '/thrift/src/test/py'
Makefile:622: recipe for target 'check-recursive' failed
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory '/thrift/src/test'
Makefile:680: recipe for target 'check-recursive' failed
make: *** [check-recursive] Error 1
travis_time:end:05c23b76:start=1642914601939318320,finish=1642916011287231557,duration=1409347913237,event=script
[0K[31;1mThe command "build/docker/run.sh" exited with 2.[0m
Done. Your build exited with 1.
```
and did hit this locally. I'd have to run the test command twice for it to pass. I think there's some CMake dependency issue. Can continue to look at it for a future PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] kainjow commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
kainjow commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1019416859
Technically this can work in older versions of Python with [enum34](https://pypi.org/project/enum34/).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] kainjow commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
kainjow commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1058796782
> We do have plan to drop support for python 2, or python3 before 3.4 (as they are all EOL now), so if we do this after that we don't really have to do any special handling and can just support it blindly. but doing it now means that we still need to handle the case of <3.4 that `IntEnum` is not available?
Yes that's right. But it might also be good to keep this as an option when introduced, and then in a later version make it the new default just to make sure it's stable.
> I'm pretty sure these 2 travis failures do not show before this pr:
>
> * https://app.travis-ci.com/github/apache/thrift/jobs/556647214
> * https://app.travis-ci.com/github/apache/thrift/jobs/556647215
ah, I only updated the CMake tests, looks like there's also equivalent for Make.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2489: THRIFT-2059: Support for Enums in Python >= 3.4
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2489:
URL: https://github.com/apache/thrift/pull/2489#issuecomment-1059800069
Nice. Didn't notice that it was 1000 commits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org