Skip to content

Document most common signals#19245

Merged
vstinner merged 3 commits intopython:masterfrom
vstinner:signals_doc
Mar 31, 2020
Merged

Document most common signals#19245
vstinner merged 3 commits intopython:masterfrom
vstinner:signals_doc

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Mar 31, 2020

Document individual signals (only the most common signals):
description, default action, availability.

Document individual signals (only the most common signals):
description, default action, availability.
@vstinner
Copy link
Copy Markdown
Member Author

@gpshead, @serhiy-storchaka, @pablogsal, @pitrou, @methane: Would you mind to review this doc change?

Does it sound a good idea to you to document the "default action"? The exact behavior might depend on the platform, but IMO it's better to have a little bit inaccurate documentation than no documentation at all.

I can create an issue if someone considers that it's needed (I added skip issue label).

@pablogsal
Copy link
Copy Markdown
Member

pablogsal commented Mar 31, 2020

I am uneasy documenting the "default" action as you mentioned it can be platform-dependent and is easy to diverge in the future. Also, these default actions are not linked to the signal themselves, is just that the kernel default action on unregistered signals is to kill the process. There is no semantic meaning for instance between SIGUSR1 and killing the process.

but IMO it's better to have a little bit inaccurate documentation than no documentation at all.

I think is better to not say anything than to say something incorrect ;)

Only SIGINT and SIGPIPE have portable default action.
@vstinner
Copy link
Copy Markdown
Member Author

I am uneasy documenting the "default" action as as you mentioned it can be platform dependent and is easy to diverge in the future.

Ok, I remove the default action, except for SIGINT and SIGPIPE: Python has a portable behavior for these two signals, since we install our own signal handler.

@vstinner
Copy link
Copy Markdown
Member Author

Is it ok to backport such documentation enhancement to Python 3.7 and 3.8?

@pablogsal
Copy link
Copy Markdown
Member

Is it ok to backport such documentation enhancement to Python 3.7 and 3.8?

I am +1 with the backport

@pablogsal
Copy link
Copy Markdown
Member

Ok, I remove the default action, except for SIGINT and SIGPIPE: Python has a portable behavior for these two signals, since we install our own signal handler.

👌

Comment thread Doc/library/signal.rst

Kill signal.

It cannot be caught, blocked, or ignored.
Copy link
Copy Markdown
Member

@pablogsal pablogsal Mar 31, 2020

Choose a reason for hiding this comment

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

<pedantic> It will be "ignored" if the process is in uninterruptible sleep (under some definition of "ignored")</pedantic>

Copy link
Copy Markdown
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks for this useful addition! 🎉

@miss-islington
Copy link
Copy Markdown
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 400e1dbcad93061f1f7ab4735202daaa5e731507 3.8

@bedevere-bot
Copy link
Copy Markdown

GH-19257 is a backport of this pull request to the 3.8 branch.

vstinner added a commit that referenced this pull request Mar 31, 2020
Document individual signals (only the most common signals):
description, default action, availability.

(cherry picked from commit 400e1db)
vstinner added a commit that referenced this pull request Mar 31, 2020
Document individual signals (only the most common signals):
description, default action, availability.

(cherry picked from commit 400e1db)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants