[tor-bugs] #11309 [Tor Support]: Improve how support statistics are reported

[Tor Bug Tracker & Wiki]

#11309: Improve how support statistics are reported
-----------------------------+----------------------------
     Reporter:  Sherief      |      Owner:  Sherief
         Type:  task         |     Status:  needs_revision
     Priority:  normal       |  Milestone:
    Component:  Tor Support  |    Version:
   Resolution:               |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+----------------------------

Comment (by lunar):

 Replying to [comment:10 Sherief]:
 > Replying to [comment:9 lunar]:
 > > > > > {{{
 > > > > > commit e4e8321e12fb1149e2d92ea67ef1a943bf708343
 > > > > > Date:   Wed Jun 4 22:12:15 2014 +0300
 > > > > >
 > > > > > pups/home now reports personal user stats
 > > > > > […]
 > > > > > +    for token in tokens:
 > > > > > +        if token.expires_at > token.created_at:
 > > > > > +            data[wiki:'live_tokens' live_tokens] += 1
 > > > > > +        elif token.expires_at < token.created_at:
 > > > > > +          data[wiki:'expired_tokens' expired_tokens]  += 1
 > > > > > +        if token.created_at == token.expires_at:
 > > > > > +            data[wiki:'revoked_tokens' revoked_tokens]  += 1
 > > > > > }}}
 > > > > >
 > > > >
 > > > > This does not feel right. You are duplicating logic here. What if
 the rules for expiry change one day?
 > > >
 > > > This code is only intended for the assistant's personal statistics.
 > >
 > > Doesn't matter. You are duplicating logic in two different places.
 That's bad. It should be factored out and the common function/method used
 where needed.
 >
 >
 > Lets say if I remove token.expired_at entirely and calculated the expiry
 date on the fly using timezone() and settings.CONFIG['expiry_days'], would
 that be ok? (project wide)

 No. Exchanging code for a configuration file is not interesting here. At
 least until there are more users to the code. And then, it would probably
 still be better to use plain Python.

 > I am also puzzled by "You are duplicating logic in two different
 places", where exactly am I duplicating logic?

 This is just a method to report numbers. But this method contain logic
 which
 decide “Is this token live?”, “Was this token revoked?”. That logic does
 not
 belong in a reporting function.

 Also, now that I read it more careful, I don't see why you are testing 3
 cases when the last one is implied by the 2 previous ones.

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