[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 atagar):

 > Sounds good, the changes i might need to do must be done before April
 and i'll try to do them even earlier, before we start collecting the
 bandwidth files.

 Great! Sounds like a fine plan. Keeping things as-is until I hear
 otherwise.

 > header attribute doesn't return timestamp, should it rather return a key
 timestamp and value timestamp?

 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.

 > 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...

 {{{
 for measurement in bwfile.measurements.values():
   ... do stuff...
 }}}

 ... or get the measurement of a specific relay...

 {{{
 myMeasurement =
 bwfile.measurements.get('3BB34C63072D9D10E836EE42968713F7B9325F66')

 if myMeasurement:
   print('My bandwidth authority measurement was: %s' % myMeasurement)
 else:
   print('My relay was not measured by the bandwidth authorities')
 }}}

 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.

 > 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...

 https://stem.torproject.org/tutorials/mirror_mirror_on_the_wall.html#can-i
 -create-descriptors

 > timestamp is not required. Is there a way to pass it?

 Yup, provide 'timestamp' as the key. This is included in the pydoc
 example...

 https://stem.torproject.org/api/descriptor/bandwidth_file.html#stem.descriptor.bandwidth_file.BandwidthFile.content

 > 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.

 > 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.

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