Skip to content

Conversation

@CJCombrink
Copy link
Contributor

@CJCombrink CJCombrink commented Nov 5, 2025

Fix issues as picked up by the MSVC build

Note: The disabling of the shared lib is done due to THRIFT-5898 and should be fixed by #3226 whereby the build will be changed again

  • Did you create an Apache Jira ticket? THRIFT-5899
  • 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.

- PR#2929 called out that the changes breaks Python 3.5 since types came in in 3.6
- Python 3.6 errors out with 'from __future__ import annotations' since it looks like it was only added in 3.7
- Getting `raise TApplicationException(TApplicationException.MISSING_RESULT, "testEnum failed: unknown result"` error
- PR#2825 state it is a breaking change, not sure why and for what version of Python
- Appveyor error: ` AttributeError: module 'ssl' has no attribute 'PROTOCOL_TLS_CLIENT'`
@CJCombrink CJCombrink marked this pull request as ready for review November 5, 2025 13:20
@CJCombrink
Copy link
Contributor Author

CJCombrink commented Nov 6, 2025

@Jens-G @fishy

I have just logged https://issues.apache.org/jira/browse/THRIFT-5900, will try to see if I can do something to get past it without trying to upgrade python. Don't want to now have to mix this PR with also upgrading to Python 3.14

@CJCombrink CJCombrink force-pushed the THRIFT-5899_python_appveyor branch from d804449 to b0e704b Compare November 7, 2025 05:40
@CJCombrink
Copy link
Contributor Author

@fishy Thanks for your inputs and review, I think I have addressed all feedback.

I think we can merge, I propose leaving the BUILD_SHARED_LIBS: OFF as part of this PR and then I will enable again as part of the fix for THRIFT-5898, that way at least Appveyor builds should stay green for any other PRs

@fishy fishy merged commit e720e6f into apache:master Nov 7, 2025
25 of 32 checks passed
@fishy
Copy link
Member

fishy commented Nov 7, 2025

@CJCombrink thank you!

@CJCombrink CJCombrink deleted the THRIFT-5899_python_appveyor branch November 7, 2025 09:21
@CJCombrink
Copy link
Contributor Author

Given that there is now an expectation for the appveryor builds to pass is there a way to put more strict check on it not getting broken again by PRs to prevent this type of regressions in future?

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.

2 participants