[metrics-bugs] #21272 [Metrics]: Onionperf deployment

[Tor Bug Tracker & Wiki]

#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 iwakeh):

 From reading the git diffs, I have a few questions and suggestions:

 Does this code reflect the transition period of processing both
 torperf and onionperf files?  Is that the reason for introducing
 the empty property with the meaning of 'nothing to download'?

 Surely tests will be added for the new functionality of `Configuration`'s
 `getStringArray` and `getStringArrayArray` methods?
 I assume current code that uses these methods relies on receiving a non-
 empty
 array or double array.  It needs to be verified that we don't run
 into trouble in the existing code using these methods.

 All storing of files should be done by the persist-mechanism, i.e.,
 a `o.t.c.persist.TpfPersistence` class needs to be introduced (this would
 shorten `TorperfDownloader` quite a bit and prevent another
 re-implementation of this functionality).
 And, this is a prerequisite for also syncing these files with
 CollecTor's sync-mechanism, which was left out because we knew
 a Torperf replacement is coming.

 It might be useful to put the Configuration's getUrlArray method
 to work.  The old Torperf code wasn't modernized, b/c we knew
 it will be replaced.  Now, it might be better to have
 OnionPerfHosts as Url array property.  This way the url-property
 checking in the downloader class is not necessary anymore as
 `Configuration`
 already provides it.
 Why is the host-nick-name needed and could not be replaced by the hostname
 itself?  (For torperf it is used for further configuration.)  Or, is this
 one of the rough edges and note yet available?
 If so, maybe have a different configuration approach instead of the
 hybrid-String-Url array?
 It would be great to make the configuration simpler to read
 and edit and keep allmost all property-checking code in the
 `Configuration` class.  For example, an url-array and a string-array-array
 property, the latter for the configuration similar to torperf.
 The only check left for the TorperfDownloader would be to match
 length of these two array- properties, but there might be other
 good approaches.

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