Re: [tor-bugs] #7152 [Stem]: Implement Controller.attach_stream

["Tor Bug Tracker & Wiki" <torproject-admin@xxxxxxxxxxxxxx>]

#7152: Implement Controller.attach_stream
--------------------+-------------------------------------------------------
 Reporter:  neena   |          Owner:  atagar        
     Type:  task    |         Status:  needs_revision
 Priority:  normal  |      Milestone:                
Component:  Stem    |        Version:                
 Keywords:          |         Parent:                
   Points:          |   Actualpoints:                
--------------------+-------------------------------------------------------
Changes (by atagar):

  * status:  new => needs_revision


Comment:

 Hi Ravi. Glad you're back! I'll resume sending you code reviews, I should
 have something soonish (didn't make sense to continue interrupting you
 while you were on your trip - hope it was fun).

 Change looks good...

 > +    response = self.msg("ATTACHSTREAM %s %s%s" % (str(stream),
 str(circuit), hop_str))

 The str() calls aren't necessary...

 {{{
 >>> "number %s" % 5
 'number 5'
 }}}

 It doesn't make a difference though for clarity I'd probably use %i...

 {{{
 >>> "number %i" % 5
 'number 5'
 }}}

 > +      time.sleep(5) # wait for the circuit to be built.
 > +      # TODO: This is fragile. It doesn't check if the circuit was
 successfully
 > +      # created. We should use the CIRC events once it exists

 Sleep should be avoided in tests if at all possible. Somewhat
 hypocritically here's an example of listening for raw events in tests that
 have a sleep...

 https://gitweb.torproject.org/stem.git/blob/HEAD:/test/integ/control/base_controller.py#l129

 > +      circs = [circuit.split(" ") for circuit in controller.get_info
 ("circuit-status").split("\n")]
 > +      exit_circ = filter(lambda circuit: circuit[0] is circuit_id,
 circs)[0]
 > +      exit_relay_id = exit_circ[2].split(",")[-1][1:41]

 We should probably add a nicer method for this, like arm's
 torTools.getCircuits...

 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torTools.py#l1051

 > +      # Get our exit relay's IP address from the descriptor
 > +      exit_ip = [line for line in controller.get_info("ns/id/" +
 > +        exit_relay_id).split("\n") if line.startswith("r ")][0].split("
 ")[-3]

 Unnecessarily icky, please use get_network_status()...

 https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/control.py#l701

 > +      # Helper function to get the result of a threaded function call
 > +      def thread_helper(func, result, *args, **kwargs):
 > +        result[0] = func(*args, **kwargs)
 ...

 Hmmm, there must be a nicer way of doing this but I'm not sure what it is
 at the moment. I'm presently working on stem's tutorials and in one of
 those I want to exemplify using stem for client traffic so I'll try to
 look into making this more elegant.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7152#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs