[tor-bugs] #6114 [Stem]: Implement GETCONF parsing in Stem

[Tor Bug Tracker & Wiki]

#6114: Implement GETCONF parsing in Stem
--------------------+-------------------------------------------------------
 Reporter:  neena   |          Owner:  neena         
     Type:  task    |         Status:  needs_revision
 Priority:  normal  |      Milestone:                
Component:  Stem    |        Version:                
 Keywords:          |         Parent:                
   Points:          |   Actualpoints:                
--------------------+-------------------------------------------------------
Changes (by atagar):

  * status:  needs_review => needs_revision


Comment:

 > +    :param bool multiple: if True, the value(s) provided are lists of
 all returned values,
 > +                          otherwise this just provides the first value

 Sounds good, but lets have the get_conf() handle the 'multiple' flag.
 There's really little need for this to influence how we parse the
 response.

 We will have three use cases for GETCONF...

 a. Simple use case of querying a single item. This will be the majority
 use case, and is what arm's getOption() method does. Here we *only* want
 either a string or a list of strings.

 b. Request for a mapping value, of which HiddenServiceOptions is the only
 one that exists at present. This is what arm's getOptionMap() is for and
 we're still missing this capability in stem.

 c. Batch request, which like getOptionMap() produces a dictionary of
 'option => value' mappings. This is something that arm did not handle, but
 we should. This can be nicely melded with the getOptionMap() method.

 I would suggest that we add two methods to the controller...

 {{{
 1. get_conf with parameters...
   - param (str)
   - default (object)
   - multiple (bool)

 2. get_conf_map with parameters...
   - param (str or list for batch call)
   - default (object)
 }}}

 Thoughts? Also please note that arm's getOption() is actually pretty smart
 about querying mapped values (like HiddenServiceDir). The more I look at
 it, the more I like how arm was handling this...

 > Ah. I've removed this check entirely. Were you suggesting something
 else?

 On reflection, the check is appropriate for the first of those two
 methods.

 > I'm thinking it would be better to this if/when we have a hidden service
 target?
 > If you want to add this to BASE_TORRC, I could do that (quickly).

 Are hidden service options something that we can setconf to enable, test
 against, then resetconf to revert them? If not, then maybe we should add a
 RUN_HIDDEN_SERVICE target for the integ tests.

 HiddenServiceOptions is a weird use case so we really should include it in
 both our integ and unit tests.

 > Maybe doing deduplication isn't such a good idea.

 Agreed. On reflection my suggestion to use a set is also fraught with
 peril since we'll need to retain the ordering for list values (oops).

 > There are other things that could make a request invalid, such as this
 in SETCONF responses.

 Nice catch.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/6114#comment:10>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online