Re: [tor-bugs] #21272 [Metrics]: Onionperf deployment

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#21272: Onionperf deployment
-------------------------+------------------------------
 Reporter:  hiro         |          Owner:  metrics-team
     Type:  enhancement  |         Status:  needs_review
 Priority:  Medium       |      Milestone:
Component:  Metrics      |        Version:
 Severity:  Normal       |     Resolution:
 Keywords:               |  Actual Points:
Parent ID:               |         Points:
 Reviewer:               |        Sponsor:
-------------------------+------------------------------

Comment (by karsten):

 Replying to [comment:25 iwakeh]:
 > Replying to [comment:23 karsten] and what's left from [comment:18]:
 > > iwakeh, please find my updated task-21272 branch with some tweaks as
 discussed above.
 >
 > Thanks, for these changes!
 >
 > Couldn't `downloadFromOnionPerfHost` do some of the filename checking
 before
 > calling `downloadAndParseOnionPerfTpfFile`?

 Well, that wouldn't change functionality but would be a simple
 refactoring, right?  What's the goal there?  Make methods more testable or
 easier to read or something else?  In any case, would you want to suggest
 new methods, and I'll move around code?  Or do you want to work on a
 patch?

 > About the older code:
 > Could we just also make the change for #20514 now?
 > In addition, there are quite a few places where try-with resources
 > or some of `Files`' methods would prevent unclosed readers/writers.
 > Still this older code needs even more work, sigh.  Maybe, after
 Amsterdam?

 No need to spend time on this.  Let's just remove the Torperf code in a
 few weeks when we're certain that the OnionPerfs can take over.

 > And, regarding the descriptor parsing don't the checks inside
 [https://gitweb.torproject.org/karsten/metrics-
 db.git/tree/src/main/java/org/torproject/collector/torperf/TorperfDownloader.java?h=task-21272&id=3318eb8ca769392cca1a6ddc3344c43eba62da91#n797
 this loop] belong into descriptor/metrics-lib itself?
 > (that might be a new ticket, though)

 Well, if we moved that code to metrics-lib, users wouldn't be able to read
 Torperf results from anything else than the originally named .tpf file.
 We usually avoid dependencies on file names if we can.  This case is a bit
 different, because we're archiving .tpf files, and we should be certain
 that they contain what they say.  I'd say that's specific to the CollecTor
 case though and cannot be generalized in metrics-lib.

 Note that we could have picked a different approach by parsing descriptors
 from files and appending them to new files with file names taken from
 descriptor contents.  The result might be the exact same output, or it
 could be a file with fewer descriptors or descriptors in a different
 order, etc.  But I felt it's easier to verify .tpf file contents and
 either take them or leave them.

 > I can take a look at the Persistence topic at some point.

 Sounds good!  The last paragraph above might be relevant here.  Hope this
 works with the persistence classes.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/21272#comment:26>
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