Opened 8 years ago

Closed 8 years ago

Last modified 3 years ago

#8165 closed defect (fixed)

torspec: Document the new "flag-thresholds" item in votes

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-spec
Cc: Actual Points:
Parent ID: #8151 Points:
Reviewer: Sponsor:


Child Tickets

Change History (12)

comment:1 Changed 8 years ago by asn

Status: newneeds_review

See branch bug8165 in

comment:3 Changed 8 years ago by atagar

Please add a formal description of the Thresholds attribute. At present there's not enough information to know what the line has. My first thought was 'oh, they're probably just space separated flags', then 'hm, maybe they're key=<int> mappings?' - both were wrong so I was lucky that Karsten had an example on #8164.

Something like the following might work...

"flag-thresholds" SP THRESHOLDS NL

[At most once for votes; does not occur in consensuses.]

THRESHOLD_KEY = [A-Za-z0-9]+

I suspect that what we really want for THRESHOLD_VALUE is 'ArgumentChar except WS' though I'm not entirely sure how we'd define that.

Also, can there be duplicate THRESHOLD_KEYs? If so then what does it mean?

comment:4 Changed 8 years ago by atagar

Correction, THRESHOLD_KEY should probably be 'KeywordChar+'.

comment:5 Changed 8 years ago by atagar

Even better, can we ensure numeric values with an optional '%'?

THRESHOLD_VALUE = [0-9]+[%]?

If so then libraries like stem and metrics-lib can provide a nicer API to users.

comment:6 Changed 8 years ago by arma

Seems like Tor should stop including the %. It doesn't help and seems to hurt.

comment:7 Changed 8 years ago by karsten

Stem and metrics-lib could simply divide threshold values with a trailing % by 100. No need to change the line format, IMO. In fact, the format with % is already in a released version, so please don't change it.

comment:8 Changed 8 years ago by arma

To be clear, I'm not suggesting changing where the decimal place goes. Just dropping the extraneous characters that no parsers are using anyway.

comment:9 Changed 8 years ago by karsten

Still, you'd change the format by doing so. Parsers have to support all existing formats, with and without the % sign, because they have to handle archived votes, too.

comment:10 Changed 8 years ago by nickm

Milestone: Tor: 0.2.4.x-final
Resolution: fixed
Status: needs_reviewclosed

Okay, I've merged and tweaked it a little. If it's wrong somehow, more patches are welcome.

comment:11 Changed 3 years ago by teor

Keywords: tor-spec added

Consistently use tor-spec across all tickets (add tor-spec).

comment:12 Changed 3 years ago by teor

Keywords: torspec removed

Consistently use tor-spec across all tickets (remove torspec).

Note: See TracTickets for help on using tickets.