-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-36863: [C#][Packaging] Do not shutdown PythonEngine on CDataInterfacePythonTests if .NET is > 5.0 #36868
Conversation
…InterfacePythonTests if .NET is > 5.0
|
@github-actions crossbow submit nuget |
Revision: a6e6d4e Submitted crossbow builds: ursacomputing/crossbow @ actions-21edeec61b
|
@westonpace are you ok with this? |
@CurtHagenlocher does this make sense to you? I recall you mentioning shutting down python but now I can't find that comment. |
The unload is only required when not using .NET Core. Under .NET Framework, unloading the AppDomain hangs when Python.NET hasn't previously been unloaded (and Core doesn't have AppDomains). Disabling this for Core runs returns us to status quo and avoids a mysterious crash we were seeing in the release branch. |
Ah, I see your comments on the original issue too! I am fine with this change. |
…acePythonTests if .NET is > 5.0 (#36868) ### Rationale for this change Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue. ### What changes are included in this PR? Only Shutdown if `#if !NET5_0_OR_GREATER` ### Are these changes tested? Locally and via archery. ### Are there any user-facing changes? No * Closes: #36863 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit d9b9003. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
…InterfacePythonTests if .NET is > 5.0 (apache#36868) ### Rationale for this change Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue. ### What changes are included in this PR? Only Shutdown if `#if !NET5_0_OR_GREATER` ### Are these changes tested? Locally and via archery. ### Are there any user-facing changes? No * Closes: apache#36863 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
…InterfacePythonTests if .NET is > 5.0 (apache#36868) ### Rationale for this change Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue. ### What changes are included in this PR? Only Shutdown if `#if !NET5_0_OR_GREATER` ### Are these changes tested? Locally and via archery. ### Are there any user-facing changes? No * Closes: apache#36863 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
Rationale for this change
Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue.
What changes are included in this PR?
Only Shutdown if
#if !NET5_0_OR_GREATER
Are these changes tested?
Locally and via archery.
Are there any user-facing changes?
No