Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#4479 defect closed fixed (fixed)

Documentation: Process implements IProcessTransport; signalProcess accepts "TERM" arg

Reported by: jrennie Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: glyph, jessica.mckellar@… Branch: branches/iprocesstransport-signal-doc-4479
(diff, github, buildbot, log)
Author: jesstess Launchpad Bug:

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)

twisted-4479-try1.patch (13.5 KB) - added by jrennie 4 years ago.
Try #1
twisted-4479-try2.patch (1.2 KB) - added by jrennie 4 years ago.
second try

Download all attachments as: .zip

Change History (12)

comment:1 follow-up: Changed 4 years ago by exarkun

  • Keywords easy added
  • Owner changed from glyph to jrennie

Can you provide a patch for this?

Changed 4 years ago by jrennie

Try #1

comment:2 in reply to: ↑ 1 Changed 4 years ago by jrennie

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 4 years ago by exarkun

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.

Changed 4 years ago by jrennie

second try

comment:4 Changed 4 years ago by jrennie

  • Keywords review added
  • Owner jrennie 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 4 years ago by jesstess

  • Cc jessica.mckellar@… added
  • Owner set to jesstess
  • Type changed from enhancement to defect

comment:6 Changed 4 years ago by jesstess

  • Author set to jesstess
  • Branch set to branches/iprocesstransport-signal-doc-4479

(In [29289]) Branching to 'iprocesstransport-signal-doc-4479'

comment:7 Changed 4 years ago by jesstess

  • Resolution set to fixed
  • Status changed from new to closed

(In [29291]) Merge iprocesstransport-signal-doc-4479

Author: jrennie
Reviewer: jesstess
Fixes: #4479

Document that IProcessTransport accepts KILL, TERM, and INT as valid
cross-platform signals.

comment:8 Changed 4 years ago by jesstess

  • 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 4 years ago by Screwtape

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 3 years ago by <automation>

  • Owner jesstess deleted
Note: See TracTickets for help on using tickets.