[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #18910 [Metrics/CollecTor]: distributing descriptors accross CollecTor instances

#18910: distributing descriptors accross CollecTor instances
-------------------------------+---------------------------------
 Reporter:  iwakeh             |          Owner:  iwakeh
     Type:  enhancement        |         Status:  needs_review
 Priority:  High               |      Milestone:  CollecTor 1.1.0
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  ctip               |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:
-------------------------------+---------------------------------

Comment (by karsten):

 Okay, I started reviewing those seven commits, but I'm running out of
 brains here.  However, before getting back to reviewing tomorrow, I
 figured I could share my thoughts on the first of the seven commits,
 16e1c84:

  - It seems we're deprecating a few config options by adding a comment to
 the default `collector.properties` and not paying attention to those
 options in the code anymore.  Shouldn't we instead remove those config
 options entirely, so that operators notice for sure that they need to
 change these config options?  We could mention this in the change log as
 medium change.

  - Speaking of, can you include a change log entry for this commit?

  - I wonder if we could simplify the configuration by avoiding that tri-
 state SyncType option and turning it into a boolean.  Consider
 `ImportCachedRelayDescriptors`, `ImportDirectoryArchives`, and
 `DownloadRelayDescriptors` in the relaydescs module.  We could just add a
 fourth option `SyncRelayDescriptors` that can be `true` or `false`.
 Basically, syncing descriptors from other CollecTor instances would be the
 fourth source for collecting relay descriptors.  This shouldn't change
 much in the code you wrote, but it might make things a bit simpler for
 future operators.  For bridge descriptors and exit lists there would have
 to be two new options to activate the current sources, that is, sanitize
 bridge descriptors found in a local directory or download exit lists from
 the exit list server.

  - I found at least one long line that checkstyle should complain about,
 though I didn't run it myself.

 In case you want to start working on any of these comments, can you please
 write `--fixup` commits that resolve those issues in this particular
 commit?  In any case, please don't modify commits 2 to 7 in that branch at
 this point, because I already started reviewing those.

 More tomorrow.  Thanks!

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