Opened 2 months ago

Closed 6 weeks ago

#30311 closed defect (fixed)

Maybe add HeaderLine to the bandwidth file specification

Reported by: juga Owned by:
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: bandwidth-file-spec, tor-spec
Cc: metrics-team Actual Points:
Parent ID: Points: 1
Reviewer: nickm Sponsor:

Description

In #30255 karsten comments that there should be a HeaderLine ::= KeyValue NL to simplify the parser in metrics.
It's not true in the case of Timestamp NL, but maybe we can just clarify that.

Child Tickets

Change History (10)

comment:1 Changed 2 months ago by juga

Summary: Maybe add HeaderLine to the bandwidht file specificationMaybe add HeaderLine to the bandwidth file specification

Fix typo.

comment:2 Changed 2 months ago by karsten

Cc: metrics-team added

Thanks for discussing this spec change! As soon as it's merged, we can simplify the parser in metrics-lib.

comment:3 Changed 7 weeks ago by juga

Status: newneeds_review

comment:4 Changed 7 weeks ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.1.x-final

comment:5 Changed 7 weeks ago by asn

Reviewer: nickm

comment:6 Changed 7 weeks ago by nickm

I don't understand how this interacts with RelayLine -- RelayLine doesn't always match HeaderLine, but I think that RelayLine fields might be allowed?

comment:7 Changed 7 weeks ago by nickm

Status: needs_reviewneeds_revision

Also I left a note on the PR.

comment:8 in reply to:  6 Changed 6 weeks ago by juga

Replying to nickm:

I don't understand how this interacts with RelayLine -- RelayLine doesn't always match HeaderLine, but I think that RelayLine fields might be allowed?

Sorry, i don't understand, what do you mean by fields?, and by interacts?, HeaderLine does not contain spaces nor more than one KeyValue.

comment:9 in reply to:  7 Changed 6 weeks ago by juga

Status: needs_revisionneeds_review

Replying to nickm:

Also I left a note on the PR.

fixup

comment:10 Changed 6 weeks ago by nickm

Resolution: fixed
Status: needs_reviewclosed

LGTM, squashed and merged.

(My question about RelayLine was wrong -- I didn't understand that this only applied to the headers :/ )

Note: See TracTickets for help on using tickets.