[tor-bugs] #29333 [Core Tor/Stem]: Use the bandwidth-file-spec.txt keywords as BandwidthFile attributes

[Tor Bug Tracker & Wiki]

#29333: Use the bandwidth-file-spec.txt keywords as BandwidthFile attributes
---------------------------+------------------------
 Reporter:  juga           |          Owner:  atagar
     Type:  defect         |         Status:  new
 Priority:  Medium         |      Milestone:
Component:  Core Tor/Stem  |        Version:
 Severity:  Normal         |     Resolution:
 Keywords:  tor-bwauth     |  Actual Points:
Parent ID:                 |         Points:
 Reviewer:                 |        Sponsor:
---------------------------+------------------------

Comment (by juga):

 Replying to [comment:6 atagar]:

 > Would you like it to? In our spec the timestamp is a weird exception
 made for backward compatibility with TorFlow. Personally I think we should
 change it to 'timestamp=<value>' with a note saying 'in bandwidth file
 version x the 'timetamp=' key MAY not be present, but MUST be present in
 future versions'. But clearly not a big whoop.
 >
 > BandwidthFiles have a separate 'timestamp' attribute but you're right
 that I don't include it in the 'header' dictionary.
 >

 While is an idea to have timestamp also as a key-value[*] at some point,
 it'd be not backward compatible with current Tor versions, so probably
 won't be changed any time soon.
 If the change would require that sbws creates bandwidth files with
 timestamp as key-value too, then we can't do this change yet.

 > > measurements attribute returns a dictionary, should it rather return a
 list? (the fingerprint is in node_id key)
 >
 > Structuring this as a dictionary gives more flexibility. Callers can
 process the measurements as a list...
 [...]
 > Modeling this as a list prevents us from doing the later in constant
 time. Other descriptor objects (such as the Consensus class) model its
 entries as a 'fingerprint => record' dictionary for this reason too.

 It's more logical a list according to the bandwidth file specification,
 but i agree that's a good reason and it's the one we use in sbws to also
 converted to dictionary in some parts of the code.
 So fine like dictionary.

 > > When using create (should it have an alias from_dict?):
 >
 > That's an interesting idea but there's actually two different methods:
 create() and content(). These are methods of our base Descriptor class...
 [...]

 Again thinking on my code/spec logic, not on existing descriptor code.
 It's fine then :)

 [...]
 > > should the headers be passed in a key header to be consistent with
 header attribute?
 >
 > I'd rather keep create() and content() as consistent with other
 descriptor types as I can. Timestamps and content are already weird
 unavoidable exceptions.

 Fair enough..
 >
 > > should content be named measurements to be consistent with
 measurements attribute?
 >
 > Their differing types would probably cause confusion. Measurements is a
 parsed dictionary whereas content are raw lines. Their types differ.

 ok.

 [*] The idea long term is to format bandwidth files as other descriptor
 documents (no key-values), so maybe take it into account if you need to
 change the parser.

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