[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #22141 [Metrics/metrics-lib]: Deprecate `DescriptorFile` and add relevant information to `Descriptor`
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs