Opened 4 years ago

Closed 4 years ago

#7687 closed enhancement (implemented)

RFC: add close_stream method to Controller

Reported by: robinson Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords: controller
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Damian,

I have finally settled on what I think is a reasonable approach to StreamClosureReason (SCR) and RelayEndReason (RER) enums. But I am still working on tests for close_stream, so RFC for now.

1) Does my handling of the *Reason enums work for you? My research in the tor source and torspec indicates that the reasons the tor process sends in the STREAM event are direct mappings of the reason the relay ended. Except, with STREAM (FAILED|CLOSED|DETACHED), REASON=END and the "real" reason is in REMOTE_REASON.

2) How do you feel about adding the SocksiPy <http://sourceforge.net/projects/socksipy/> module under test/ ? This is a BSD-licensed module that I hope will make stream-using integ tests possible. But I have not yet tried, so maybe it won't help.

This work is in the exp-close-stream-v1 branch on git://gitorious.org/stem-robinson/stem-robinson.git and commit logs can be read at https://gitorious.org/stem-robinson/stem-robinson/commits/exp-close-stream-v1

Child Tickets

TicketTypeStatusOwnerSummary
#7859enhancementclosedatagaradd Controller.get_streams method

Change History (7)

comment:1 Changed 4 years ago by atagar

Hi Sean. Generally I like it. I've pushed some additional tweaks to the 'exp-close-stream-v1' branch of my personal repo (git://git.torproject.org/user/atagar/stem.git). Thoughts?

https://gitweb.torproject.org/user/atagar/stem.git/shortlog/refs/heads/exp-close-stream-v1

2) How do you feel about adding the SocksiPy < http://sourceforge.net/projects/socksipy/> module under test/ ?

Generally I'd like to avoid it, but there is precedent...

https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/util/ordereddict.py

Rather than bundling it how about we make it optional, skipping tests that use it if it's not present?

comment:2 Changed 4 years ago by robinson

Damian,

I agree with all your changes. I have made minor changes to your changes and pushed it to my exp-close-stream-v2 branch.

If you are okay with no tests for close_stream, then please pull this feature into stem main. I am still trying to figure a way to write integ tests for traffic going through the tor server.

https://gitorious.org/stem-robinson/stem-robinson/commits/exp-close-stream-v2

comment:3 Changed 4 years ago by atagar

Good catch about the 'NONE' enumeration. Pushed that feature branch and keeping this open to track the tests.

comment:4 follow-up: Changed 4 years ago by atagar

  • Keywords stream removed

Hi Sean. What are your plans regarding this? Is this something that I should take over?

comment:5 in reply to: ↑ 4 Changed 4 years ago by robinson

Replying to atagar:

Hi Sean. What are your plans regarding this? Is this something that I should take over?

I think all the pieces are in place that I can finish this up. I'll rebase on the post-SOCKS5-and-get_socks_listeners() tree and submit tests this week.

comment:6 Changed 4 years ago by robinson

I have the test[0] for this method, finally. Of course, I decided to add another piece of infrastructure (#7859) to aid this test. If the child ticket for this is accepted, please pull.

[0]: https://gitorious.org/stem-robinson/stem-robinson/commits/exp-close-stream-tests

comment:7 Changed 4 years ago by atagar

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

Looks great! Pushed.

Note: See TracTickets for help on using tickets.