#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
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs