#4479 defect closed fixed (fixed)
Documentation: Process implements IProcessTransport; signalProcess accepts "TERM" arg
| Reported by: | Jason Rennie | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | core | Keywords: | |
| Cc: | Glyph, jesstess | Branch: |
branches/iprocesstransport-signal-doc-4479
branch-diff, diff-cov, branch-cov, buildbot |
| Author: | jesstess |
Description
t.i.Process (or t.i._BaseProcess) should be listed as an IProcessTransport implementation.
IProcessTransport only lists "HUP", "KILL", "STOP", and "INT" as valid cross-platform signals, yet the regular/unix and win32 Process implementations also support "TERM".
See also #2838 and #1478. #1478 also notes that Process should be marked as implementing IProcessTransport.
Attachments (2)
Change History (12)
comment:1 follow-up: 2 Changed 8 years ago by
| Keywords: | easy added |
|---|---|
| Owner: | changed from Glyph to Jason Rennie |
comment:2 Changed 8 years ago by
Replying to exarkun:
Can you provide a patch for this?
Just attached. Might be a bit overzealous---I can attach a more lightweight patch if so. I also added IFileDescriptor, IReadDescriptor to PTYProcess; added IConsumer to Process; added IPushProducer to the win32 Process; noted interface methods; and grouped methods by interface.
comment:3 Changed 8 years ago by
The actual Windows implementation seems to support only INT, TERM, and KILL. So both the original version of the signal list and the new one seem to be wrong. Why shouldn't the docs just say INT, TERM, and KILL?
Also, yes, please limit the patch to fixing the issue described by the ticket. If there are other issues, feel free to file more tickets.
And when you think a ticket is ready for review, add the "review" keyword and re-assign the ticket to no one. See ReviewProcess.
comment:4 Changed 8 years ago by
| Keywords: | review added |
|---|---|
| Owner: | Jason Rennie deleted |
"second try" attachment is a minimal patch which adds IProcessTransport implementation to t.i.Process and updates IProcessTransport.signalProcess doc to only list INT, TERM, and KILL as cross-platform signals.
comment:5 Changed 8 years ago by
| Cc: | jesstess added |
|---|---|
| Owner: | set to jesstess |
| Type: | enhancement → defect |
comment:6 Changed 8 years ago by
| Author: | → jesstess |
|---|---|
| Branch: | → branches/iprocesstransport-signal-doc-4479 |
(In [29289]) Branching to 'iprocesstransport-signal-doc-4479'
comment:7 Changed 8 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:8 Changed 8 years ago by
| Keywords: | easy review removed |
|---|
Thanks for the patch, jrennie!
Process doesn't implement getPeer, which is part of the ITransport interface inherited by IProcessTransport, so I only applied the part of the patch affect IProcessTransport. Please open a separate ticket about the interface, if you'd like.
If Process did end up implementing IProcessTransport it should get a test case for that (grep for verifyObject for examples).
Also note that you don't have to escape quotes in the epytext markup.
comment:9 Changed 8 years ago by
Process doesn't implement getPeer, which is part of the ITransport interface inherited by IProcessTransport, so I only applied the part of the patch affect IProcessTransport. Please open a separate ticket about the interface, if you'd like.
Filed as #4585.
comment:10 Changed 7 years ago by
| Owner: | jesstess deleted |
|---|
Can you provide a patch for this?