Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

WIP: 449 Py_Initialize_SignalConfiguration#450

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
rmadsen-ks wants to merge9 commits intopythonnet:masterfromrmadsen-ks:449_Py_Initialize_SignalConfiguration
Closed

WIP: 449 Py_Initialize_SignalConfiguration#450

rmadsen-ks wants to merge9 commits intopythonnet:masterfromrmadsen-ks:449_Py_Initialize_SignalConfiguration

Conversation

rmadsen-ks
Copy link
Contributor

@rmadsen-ksrmadsen-ks commentedApr 5, 2017
edited
Loading

Note: This PR is not ready yet. I just added the PR to see if it builds.

What does this implement/fix? Explain your changes.

Avoiding overriding signals from the application so that CTRL-C might be handled.

Using Py_InitializeEx(0) (instead Py_Initialize) to avoid overriding signals.

Does this close any currently open issues?

Thisfixes#449

...

Any other comments?

...

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself toAUTHORS
  • Updated theCHANGELOG

@mention-bot
Copy link

@rmadsen-ks, thanks!@vmuriart,@davidanthoff,@yagweb,@filmor and@tonyroberts, please review this.

@codecov
Copy link

codecovbot commentedApr 5, 2017
edited
Loading

Codecov Report

Merging#450 intomaster willdecrease coverage by1.08%.
The diff coverage is66.66%.

Impacted file tree graph

@@            Coverage Diff             @@##           master     #450      +/-   ##==========================================- Coverage    77.4%   76.31%   -1.09%==========================================  Files          63       62       -1       Lines        5589     5569      -20       Branches      892      888       -4     ==========================================- Hits         4326     4250      -76- Misses        970     1026      +56  Partials      293      293
FlagCoverage Δ
#setup_linux63.66% <ø> (-5.76%)⬇️
#setup_windows75.66% <66.66%> (-0.92%)⬇️
Impacted FilesCoverage Δ
src/runtime/runtime.cs90.58% <0%> (-0.79%)⬇️
src/runtime/pythonengine.cs77.87% <100%> (-0.45%)⬇️
src/runtime/constructorbinding.cs9.85% <0%> (-29.58%)⬇️
src/runtime/CustomMarshaler.cs47.94% <0%> (-21.92%)⬇️
src/runtime/overload.cs58.33% <0%> (-15.58%)⬇️
src/runtime/pyiter.cs63.63% <0%> (-13.64%)⬇️
src/runtime/iterator.cs81.81% <0%> (-7.08%)⬇️
setup.py83.45% <0%> (-3.96%)⬇️
src/runtime/pyobject.cs38.85% <0%> (-0.92%)⬇️
... and7 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updatefb84cd2...24ab0d1. Read thecomment docs.

/// </remarks>
public static void Initialize(IEnumerable<string> args, bool setSysArgv = true)
public static void Initialize(IEnumerable<string> args, bool setSysArgv = true, int initSigs = 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

initSigs should be abool.

@rmadsen-ks
Copy link
ContributorAuthor

@denfromufa, it seems some of the builds takes an extraordinary amount of time (59 min). Is there some way of canceling the builds so we dont have to wait for timeout?

@den-run-ai
Copy link
Contributor

@rmadsen-ks you may need to have access to CI build servers to cancel the builds, what do you think@vmuriart ?

@den-run-ai
Copy link
Contributor

@rmadsen-ks@filmor@vmuriart do you guys agree on removing the test for this behavior and merge pull request without the tests in CI? We can leave the original issue open, until proper testing is added.

Here is a user in need of this pull request:

#511

@rmadsen-ks
Copy link
ContributorAuthor

@denfromufa, unfortunately I dont have the bandwidth to figure how to make a unit test that works reliably. It turned out to be harder than expected to signal CTRL+C to an application running in the console on windows. Also I had decided to use CSharpProvider for runtime assembly generation, which I think is not compatible with .NET core, so that will probably need to move to Roslyn.

Do you have another idea of how to test this in an automated way?

In any case, the non-test part I think is low risk, but I suggest you do a bit of manual testing first anyway.

@den-run-ai
Copy link
Contributor

I did not understand the part about CSharpProvider :)

But the Ctrl+c keyboard interrupt can be emulated with this threading call:

thread.interrupt_main() # raises KeyboardInterrupt

More details in this new issue that I linked above.

@rmadsen-ks
Copy link
ContributorAuthor

@denfromufa,thread.interrupt_main() does not work in our case because we have a C# console application running embedded python code.

This is why I wanted to use CSharpProvider. I wanted to build a short console application that delays for a few s, but is interrupted by a keyboard interrupt. This I could also do by adding an assembly to the git repo, but I figured that would not be a popular decision. In any case, i have not been able to emulate CTRL+C on windows.

In my testing running _thread.interrupt_main() (python 3) as the embedded python script does not have the desired effect.

@filmorfilmor self-assigned thisJul 23, 2018
@filmorfilmor added this to the2.4.0 milestoneJul 23, 2018
@filmor
Copy link
Member

Let's continue work on this one in#724.

@filmorfilmor closed thisAug 30, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@filmorfilmorfilmor left review comments

Assignees

@filmorfilmor

Labels
None yet
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.

Call to Py_Initialize changes signal handler configuration
4 participants
@rmadsen-ks@mention-bot@den-run-ai@filmor

[8]ページ先頭

©2009-2025 Movatter.jp