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

Re: [tor-bugs] #9889 [Atlas]: Add "Tshirt: yes/no" to Atlas and Globe

#9889: Add "Tshirt: yes/no" to Atlas and Globe
-----------------------------+----------------------------
     Reporter:  arma         |      Owner:  hellais
         Type:  enhancement  |     Status:  needs_revision
     Priority:  normal       |  Milestone:
    Component:  Atlas        |    Version:
   Resolution:               |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+----------------------------
Changes (by karsten):

 * status:  new => needs_revision


Comment:

 Hi Sreenatha!  Thanks for working on this script.  A few comments:

  - Want to clone the [https://gitweb.torproject.org/metrics-tasks.git
 metrics-tasks.git repository] and add your script under a new task-9889/
 directory?  We can still use your script for the Weather rewrite, but
 having it in metrics-tasks.git means it won't get lost, regardless of how
 the Weather rewrite goes.
  - Making 2 requests for details document is something we can tweak later.
 But did you consider using Onionoo's `fields` parameter to reduce response
 size of details documents?  (Note that this doesn't work for bandwidth
 documents.)
  - Considering 2 months as 60 days is perfectly fine as simplification.
  - Using `last_restarted` to calculate uptime is not a good idea.  It
 means people are encouraged not to restart their relay if they want to
 have a t-shirt.  But restarts are necessary to upgrade in case of security
 fixes.  How about you use the recently added uptime documents?  You could
 say that a relay needs to be around at least 95% in the past two months in
 order to be considered.
  - Not sure what you mean above about handling null values.  See the
 protocol specification for when Onionoo puts in `null`.
  - If I provide a non-existing fingerprint, I get an IndexError.  You
 should probably check that the `relays` object contains at least one
 object.
  - Rather than only accepting fingerprints as input, how about you use
 Onionoo's `search` parameter and accept anything as search term that
 Atlas/Globe accept?  Of course, that would mean processing possibly more
 than 1 relay at the same time.
  - There seems to be some duplicate code for fetching things from Onionoo
 and processing the result.  Maybe you could move that to a function?
  - Not sure how Pythonic this is, but usually, when you say "if x: return
 True; else return False;", the better way of saying it is "return x ==
 True;" or simply "return x;".
  - I didn't review the math behind check_tshirt() yet, but it would help
 if you could print details like computed uptime, computed average
 bandwidth, and computed result whether the relay allows exiting to port
 80.  That would be in addition to your end result why you think the relay
 qualifies for a t-shirt and why or why not.

 Again, thanks for working on this!  Looking forward to doing a second code
 review!

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