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

Re: [tor-bugs] #31558 [Metrics/CollecTor]: Process bridge pool assignments again



#31558: Process bridge pool assignments again
-------------------------------+--------------------------------
 Reporter:  karsten            |          Owner:  metrics-team
     Type:  enhancement        |         Status:  needs_revision
 Priority:  Medium             |      Milestone:
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:                     |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:  irl                |        Sponsor:
-------------------------------+--------------------------------

Comment (by karsten):

 Replying to [comment:4 notirl]:
 > Status: -> needs_revision_maybe
 >
 > There is not another "Processor" class in CollecTor. We already have:
 Annotation, Builder, Checker, Configuration, Criterium, Downloader,
 Exception, Hook, Json, Key, Main, Manager, Map, Metadata, Parser,
 Persistence, Reader, Scheduler, Type, Utils, Weblogs, Writer. That's a lot
 of concepts, can we reuse one for this? (I put a table later in this
 comment with the number of times each noun appears in the codebase.)

 The name comes from the class that we retired a few years back. We could
 pick a new one, though. What this class does is: read files from disk,
 possibly split them by bridge pool assignment list, replace bridge
 fingerprints with sanitized ones, write one file per sanitized bridge pool
 assignment. I think processor is fine, but if you prefer something else,
 let's switch.

 > The startProcessing() function is huge and there are no tests for it. ):
 This also makes the review difficult. I'm relatively confident that we're
 not going to accidentally publish a load of raw bridge fingerprints but as
 to the correctness of the code, and the reliability, I'm less confident.
 >
 > Saying this, if this huge function was working before then I'm happier
 about adding it back now and then filing tickets to break it down and add
 tests later. At least then it's not completely unknown new code.

 I agree that the method is too long. I'm currently working on breaking it
 up. It did work a few years back, but now is a good time to clean it up a
 little. Will have something to review later this afternoon.

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