[tor-bugs] #22141 [Metrics/metrics-lib]: Deprecate `DescriptorFile` and add relevant information to `Descriptor`

[Tor Bug Tracker & Wiki]

#22141: Deprecate `DescriptorFile` and add relevant information to `Descriptor`
---------------------------------+-----------------------------------
 Reporter:  karsten              |          Owner:  metrics-team
     Type:  enhancement          |         Status:  needs_review
 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):

 Replying to [comment:8 karsten]:
 > Replying to [comment:7 iwakeh]:
 > > DescriptorReaderImpl became more readable.  Maybe use
 Files.walkFileTree and an extension of SimpleFileVisitor to simplify the
 code which currently is in "readDescriptorFiles"?
 >
 > Good idea!  If you don't mind, let us focus on the interface part for
 the moment, and I'll make this implementation change later on (before we
 merge).  Okay?

 Fine, I'll add a new ticket.

 >
 > > Regarding `Descriptor.getException`:  The use case here is to
 determine, if the Descriptor object is valid and decide whether to use it
 or drop it?
 > > Then, why not add a `Descriptor.valid` method returning true or false?
 The detailed logging of exceptions could be configured if wanted in
 metrics-lib as an extra logging channel for parse exceptions (this code
 would need to be added still).
 >
 > Not quite.  The application already knows that it can't use an invalid
 descriptor, because it's not an `instanceof` whatever interface it
 expected.  It's just a `Descriptor`, but not, for example, a
 `RelayServerDescriptor`.  The application can check whether
 `getException()` returns something if it wants to log that or possibly
 even handle that exception somehow.  But it could as well ignore the
 descriptor of unknown/unexpected subinterface type.

 Ok, in that case I'd vote for the `InvalidDescriptor` interface, which
 would make the problems with the faulty descriptor object more visible.
 With the current setup `getException` is only useful for logging b/c there
 is only the mighty DescriptorParseException having the real problem hidden
 in the message text.  The logging would be better achieved by having a
 "ParseExceptionsLog" logger that can be dis- or enabled via configuration
 and will spare the client to code logging on top of metrics-lib.

 >
 > > About parsing "future or not" I have more questions than suggestions
 as it depends highly on the implementation.
 > > What do you have in mind?
 > > It would also introduce some overhead regarding the cancelling, how
 should that be handled?
 > > How should a client deal with the Futures it receives?
 >
 > Hmm, to be honest, I don't really know.  Maybe we can postpone that
 discussion, even at the risk of having to wait for 3.0.0.

 Yes, there are so many improvements planned and recently introduced, it
 might be good to let these settle and then decide later what else can be
 improved.
 >
 > > BTW DescriptorImpl: The `NoSuchAlgorithmException` should be escalated
 as runtime exceptions, b/c this is a problem with the jdk/jvm and
 shouldn't be hidden away.  I'd suggest using a private method for the two
 digest calls (please find a suggestion
 [https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-22141&id=31a2f15bdc5933cc1f240cff107e01e0fce69a8d
 here]).
 >
 > Good idea.  As stated above, I'll do this as soon as we have an
 interface we like.

 I'll add this to the ticket mentioned above.

 >
 > I just had a related thought on #22196 which I'll post there in the next
 hour or so.  That thought might affect what we're doing here, so let's
 briefly switch over there and return here after that. :)

 Already commented there :-)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22141#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online