Re: [tor-bugs] #20333 [Metrics/metrics-lib]: add descriptor digest to vote and streamline method name

[Tor Bug Tracker & Wiki]

#20333: add descriptor digest to vote and streamline method name
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  closed
 Priority:  Medium   |  Milestone:  metrics-lib 1.7.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:  implemented
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * status:  merge_ready => closed
 * resolution:   => implemented


Comment:

 Great, thanks for the review!  Cherry-picked and pushed.  Closing.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20333 [Metrics/metrics-lib]: add descriptor digest to vote and streamline method name

[Tor Bug Tracker & Wiki]

#20333: add descriptor digest to vote and streamline method name
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:  metrics-lib 1.7.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by iwakeh):

 * status:  needs_review => merge_ready


Comment:

 Looks fine, all checks and tests pass.
 The renaming makes sense; and the decision for a scheme that uses the
 algorithm name as argument could be revisited when there are more
 algorithms to choose from.

 Merge ready!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20333 [Metrics/metrics-lib]: add descriptor digest to vote and streamline method name

[Tor Bug Tracker & Wiki]

#20333: add descriptor digest to vote and streamline method name
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  metrics-lib 1.7.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * status:  new => needs_review
 * milestone:  metrics-lib 2.0.0 => metrics-lib 1.7.0


Comment:

 Alright, I worked on a possible patch to move this discussion forward:
  - All methods returning the digest of a given descriptor use a common
 scheme, `getDigestAlgEnc()`, for example, `getDigestSha1Hex()`.
  - All methods returning a digest referencing another descriptor contain a
 word or two explaining what other descriptor they are referencing, but
 they also use `AlgEnc`, for example, `getVoteDigestSha1Hex()`.

 I also added a digest method to votes.

 I did not switch to `digest()`, `digestAlgEnc()`, or `getDigest(Alg)`,
 because I found those names not in line with the other methods.  Let me
 know if you disagree, and why, and let's discuss.

 Assigning to metrics-lib 1.7.0, because this is a backward-compatible
 change that doesn't seem to be too hard to make.

 Please review [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/log/?h=task-20333 my task-20333 branch].

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20333 [Metrics/metrics-lib]: add descriptor digest to vote and streamline method name

[Tor Bug Tracker & Wiki]

#20333: add descriptor digest to vote and streamline method name
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.5.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 This could get solved by overloading with an additional parameter for the
 algorithm.
 That could also be useful for the sha*Hex methods.  So, it might be good
 to look for other candidates.

 From a developing user point of view I'd rather have these methods named
 the same and prefer a short name.  (Just having "digest()" is worth
 considering, but that is a bigger change.)  It's easier to make an api
 popular when there is a consistent and lean naming scheme.

 Some examples:
 `relayDesc.getDigest()` instead of `relayDesc.getServerDescriptorDigest()`
 `relayDesc.getDigest(Sha256)` instead of
 `relayDesc.getServerDescriptorDigest256()`
 `micro.getDigest()` instead of `micro.getMicrodescriptorDigest()`

 Of course,  there needs to be a transition and the clients' in Metrics
 products will have to be adapted.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20333 [Metrics/metrics-lib]: add descriptor digest to vote and streamline method name

[Tor Bug Tracker & Wiki]

#20333: add descriptor digest to vote and streamline method name
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.5.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by karsten):

 Somewhat related to the `sha1Hex()` example, depending on the descriptor
 we might need more than just one method for returning a descriptor digest:
 sha-1, sha-256, etc.  Of course, we could pick one name per digest
 algorithm.  But maybe we'll also need different encodings, hex vs. base64.
 This can get messy.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20333 [Metrics/metrics-lib]: add descriptor digest to vote and streamline method name

[Tor Bug Tracker & Wiki]

#20333: add descriptor digest to vote and streamline method name
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.5.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by karsten):

 Replying to [ticket:20333 iwakeh]:
 > There is currently not vote descriptor digest (so CollecTor calculates
 it on its own).  Most other descriptor types provide a digest.

 Oh, that sounds like an oversight then.  Nobody needed that yet, so it has
 not been implemented.  Let's implement it!

 > In addition, it might be nice to provide a `getDigest` method for the
 different descriptor digests instead of `XyzDescriptor.getXyzDigest`.

 What do you suggest?  Just rename the method and deprecate the old one?
 Is that worth the possible trouble for developers who need to adapt method
 names?  (I just remember how I had to change `shaHex()` to `sha1Hex()` the
 other day and found that change rather annoying.)  And having two methods
 that do the exact same thing seems rather confusing, too.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs