Re: [tor-bugs] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+--
 Reporter:  iwakeh   |  Owner:  iwakeh
 Type:  enhancement  | Status:  accepted
 Priority:  Medium   |  Milestone:
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by karsten):

 * milestone:  metrics-lib 1.9.0 =>


Comment:

 We decided today not to let this ticket (and #17861) delay the 1.9.0
 release, and 2.0.0 won't be a good target milestone either.  Taking this
 out of planned milestones for now.

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  iwakeh
 Type:  enhancement  | Status:  accepted
 Priority:  Medium   |  Milestone:  metrics-lib 1.9.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Thanks for the input!
 I'm trying to consider as many topics as possible while working on this
 with the aim to only introduce minimal changes that provide the most
 benefits.

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  iwakeh
 Type:  enhancement  | Status:  accepted
 Priority:  Medium   |  Milestone:  metrics-lib 1.9.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by karsten):

 Hmm.  The use cases you describe make sense for adding such marker
 interfaces sound plausible.  But it also seems easy to introduce these
 interfaces and potentially difficult to deprecate and remove them if we
 don't want them anymore.  Should we wait with adding them until we need
 them?

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  iwakeh
 Type:  enhancement  | Status:  accepted
 Priority:  Medium   |  Milestone:  metrics-lib 1.9.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Metrics-lib parses Tor protocol related descriptors and descriptors from
 other sources.  Shouldn't this distinction be reflected in the interface
 hierarchy?
 Have `Descriptor` on top and introduce an `ExternalDescriptor` interface
 for non-Tor protocol related data sources? Maybe, even have
 `TorDescriptor` too?
 {{{
   Descriptor
 /   \
 TorDescriptor ExternalDescriptor
 }}}


 This would provide more freedom when for example a switch to an external
 parsing framework (like ANTLR) is made for the Tor related descriptors.
 In addition, non-Tor descriptors could be modeled more easily to the soon
 to come Metrics-data standards and even have their own parsing/generator.

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  iwakeh
 Type:  enhancement  | Status:  accepted
 Priority:  Medium   |  Milestone:  metrics-lib 1.9.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by iwakeh):

 * owner:  metrics-team => iwakeh
 * status:  assigned => accepted


--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  metrics-team
 Type:  enhancement  | Status:  assigned
 Priority:  Medium   |  Milestone:  metrics-lib 1.9.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Also from #21932 comments 14/15, listed for discussion:
 * avoid constant duplication, think about adding or not adding certain
 constants to the api (refers to DescriptorImpl.NL and ExitList.EOL)
 * add overloaded methods for `newScanner`, b/c the usual delimiter is "\n"
 * DescriptorImpl.setDigestXXX allow empty or null argument.  This should
 have a check even though currently the calling methods make sure the
 argument is not null or empty, but when working on other tasks in the
 future that might not be apparent anymore and would get lost w/o a check
 and accompanying test.
 * Think about improving TorperfResultImpl and ExitListImpl (also in regard
 to delimiter use, see question 4, comment 14, #21932).

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  metrics-team
 Type:  enhancement  | Status:  assigned
 Priority:  Medium   |  Milestone:  metrics-lib 1.9.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by iwakeh):

 * milestone:  metrics-lib 2.0.0 => metrics-lib 1.9.0


--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  metrics-team
 Type:  enhancement  | Status:  assigned
 Priority:  Medium   |  Milestone:  metrics-lib 2.0.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * owner:  karsten => metrics-team
 * status:  new => assigned


Comment:

 I'd like us to flesh out how we'd like to improve the interface hierarchy,
 and I'll start by going through the ideas above.

  1. Comment 10 of #19398 says: "There shouldn't be two interfaces
 declaring the very same method; there should be a more structured
 hierarchy for the interfaces, too."  I think I agree in theory, though I'd
 want to discuss how strictly we want to implement this principle.  Here's
 a contrasting principle: let's avoid introducing interfaces that have the
 sole purpose of deduplicating methods.  I think we need to find a middle
 ground here.

  2. The linked comment in `NetworkStatusImpl` seems related to possible
 improvements to the interface hierarchy, though I'd prefer if we address
 that issue after improving the interface hierarchy.

  3. Comment 4 above, splitting `ParseHelper` into interface and
 implementation seems unrelated, unless we're planning to make the
 interface part of the metrics-lib API.  Right now it's solely an
 implementation helper to avoid duplicating code.

  4. #17861 is indeed something where we could improve the interface
 hierarchy, by having a separate interface for each descriptor type.  But
 I'm not sure how to best implement such a new interface.  See the heavy
 overlap between `RelayNetworkStatusVote` and
 `RelayNetworkStatusConsensus`.  A new
 `RelayNetworkStatusMicrodescConsensus` interface following the current
 code practice would basically copy over the entire consensus interface and
 change a method here and there.  We should avoid that.

 In addition to those items, I came up with a few more ideas:

  5. When we added `RelayServerDescriptor`, `RelayExtraInfoDescriptor`,
 `BridgeServerDescriptor`, and `BridgeExtraInfoDescriptor`, we left all
 methods in the superinterfaces `ServerDescriptor` and
 `ExtraInfoDescriptor`.  We should consider moving methods that are
 specific to relays or bridges to the subinterfaces.

  6. We're still using a single `NetworkStatusEntry` for entries in all the
 different network statuses.  We should consider using an interface
 hierarchy there, too, to only provide relevant methods depending on the
 network status at hand.  I started working on a patch, but I'd like to
 keep this discussion on the conceptual level for now.

 And here are some more concrete suggestions for improving the interface
 hierarchy, somewhat overlapping with the ideas above:

  7. Introduce a `NetworkStatus` interface to capture all common parts of
 network statuses in directory protocol versions 2 and 3, including all
 `RelayNetworkStatus*` and `BridgeNetworkStatus`.

  8. Introduce another interface called `NetworkStatusVote` for common
 parts of network statuses in directory protocol version 3, following the
 common part in URLs like `http:///tor/status-
 vote/next/authority.z`.  Though there's the risk that this interface will
 be confused with `RelayNetworkStatusVote`.  Another possible interface
 name could be `RelayNetworkStatusVersion3`, though I'm not a big fan of
 including numbers in type names.

  9. Introduce a new interface `NetworkStatus.Entry` just like
 `ExitList.Entry` and a set of subinterfaces like
 `RelayNetworkStatusConsensus.Entry` to address issue 6 above.  Requires
 some heavy generics lifting.

  10. Use `RelayNetworkStatusConsensus` for the common parts in consensuses
 of any flavor and introduce `RelayNetworkStatusNsConsensus` for unflavored
 /ns-flavored and `RelayNetworkStatusMicrodescConsensus` for microdesc-
 flavored consensuses.  That's basically what we did with adding new
 subinterfaces to `ServerDescriptor`.

 What else can/should we do?  Or which parts should we rather not do?

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 2.0.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by iwakeh):

 * type:  defect => enhancement


Comment:

 see also #17861.

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+---
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  defect   | 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):

 `ParseHelper` should be split into a utility class that is part of the
 interface and the implementation specific part inside `impl`.

--
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] #19640 [Metrics/metrics-lib]: review and improve interface hierarchy

[Tor Bug Tracker & Wiki]

#19640: review and improve interface hierarchy
-+-
 Reporter:  iwakeh   |  Owner:  karsten
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by iwakeh):

 cf. [https://gitweb.torproject.org/user/iwakeh/metrics-
 
lib.git/tree/src/main/java/org/torproject/descriptor/impl/NetworkStatusImpl.java?h=task-19398-final=ac5ff698c9585d4f5414e40cd6dd978692e8409a#n264
 comment in `NetworkStatusImpl`]

--
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