#30726: Missing relay keys in bandwidth file spec -------------------------------------+----------------------------------- Reporter: teor | Owner: (none) Type: defect | Status: needs_revision Priority: High | Milestone: sbws: 1.1.x-final Component: Core Tor/Tor | Version: Severity: Major | Resolution: Keywords: sbws-spec, sbws-roadmap | Actual Points: Parent ID: #33121 | Points: 6 Reviewer: ahf, gk | Sponsor: -------------------------------------+----------------------------------- Changes (by gk):
* status: needs_review => needs_revision Comment: Replying to [comment:11 juga]: > Replying to [comment:10 gk]: > > > > So i should check the collector to be sure about the which keys were in which versions > > > > Sounds good. > > I checked and all the collected bandwidth files started with the specification version 1.4.0, so in versions earlier than that it does not really matter the KeyValues are not correct. > > Still, i fixed them in their respective versions, including the version 1.3.0 that was never generated by sbws (it should have been other ticket, but the change is small). > > https://github.com/torproject/torspec/pull/110 Looks mostly good to me. I have some comments/nits mainly for `0b6e97ca37b60fbfbd628306ec4b0551b34142cc`: `4ec93f2d8b2add1da598d927785ed7d8e60f0ec3`: looks good `0b6e97ca37b60fbfbd628306ec4b0551b34142cc`: * `desc_bw_bur`, `consensus_bandwidth`, and `consensus_bandwidth_is_unmeasured` are all in `v1.0.3` released which is using `SPEC_VERSION` 1.2.0. Thus, those `KeyValues` are already in that spec version available * in the commit message s/They appeared in in/They appeared in/ * s/the consensus bandwdith/the consensus bandwidth/ * In the new "in the last data_period days" parts the default value for "data_period days" is not mentioned anymore as it is in previous `KeyValue` explanations. Maybe add a hint to the default one, here too * In `relay_recent_priority_list_count` there is no "in the last data_period days" but there should be, shouldn't it (at least that's what `v3bwfile.py` is indicating)? * "Add more KeyValues": I think it might make sense to use "Adds more KeyValues" in the sense that a particular spec version *adds* those things. I am a bit confused though, as the explanations to previous spec versions alternate between "Add" and "Adds" without some pattern. You could fix that part up in a follow-up commit if you think we should do that. `fb36a2bae2bc0aa1f0dfbfa68fbb903a5ee6d193`: looks good (modulo the previously mentioned "Adds"/"Add" inconsistency; could be part of a follow-up fix as well) > What i should still check is whether we need to open tickets to metrics and stem because of the change of version in the spec. I don't think so because there were no bandwidth files published earlier than 1.4.0. I agree that it should not be needed from my understanding but, yes, could be worth double-checking, though. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30726#comment:12> 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