Skip to content

Conversation

@jdtournier
Copy link
Member

When debugging long-running scripts (such as dwifslpreproc), it's sometimes useful to interrupt the process when it's already moved on from the problematic stage, but hitting Ctrl-C currently deletes the temporary folder even if the process was invoked with -nocleanup, which IMO is unexpected. This seems to fix it.

@jdtournier jdtournier added this to the 3.0.4 milestone Jun 22, 2022
@jdtournier jdtournier requested a review from Lestropie June 22, 2022 15:14
@jdtournier jdtournier self-assigned this Jun 22, 2022
Copy link
Member

@Lestropie Lestropie left a comment

Choose a reason for hiding this comment

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

Yep, keep encountering this myself but never got around to fixing it.

Outstanding question however is whether the terminal message should be exactly the same (ie. report receipt of signal), or whether text reporting the retainment of the scratch directory and its location (which gets printed upon failure / completion if -nocleanup is specified) should be recapitulated here.

Copy link
Member

@Lestropie Lestropie left a comment

Choose a reason for hiding this comment

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

This should now do as initially desired but also print the location of the scratch directory if the handler is invoked but -nocleanup` was specified.

@Lestropie
Copy link
Member

Does anybody else want to review this given I changed all of the code?

@jdtournier
Copy link
Member Author

Does anybody else want to review this given I changed all of the code?

I would, but GitHub won't let me as I'm the originator for this pull request... But I have looked at the code, and I approve!

@jdtournier jdtournier merged commit 8b787e5 into master Nov 2, 2022
@jdtournier jdtournier deleted the python_nocleanup_signal branch November 2, 2022 15:03
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.

3 participants