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

Re: [tor-bugs] #21236 [Metrics/Metrics website]: Put a visualization of Tor Browser downloads and updates on the Metrics website

#21236: Put a visualization of Tor Browser downloads and updates on the Metrics
website
-------------------------------------+------------------------------
 Reporter:  karsten                  |          Owner:  karsten
     Type:  enhancement              |         Status:  needs_review
 Priority:  High                     |      Milestone:
Component:  Metrics/Metrics website  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:                           |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+------------------------------

Comment (by iwakeh):

 Replying to [comment:20 karsten]:
 > Thanks for refactoring this code.  I'll have to look at your patch more
 closely.  But I'd rather want to do after publicly deploying this new
 graph.  On a first look I don't see any harm in splitting up existing code
 into multiple classes.  But I also don't see a clear benefit that would
 urge us to make the change immediately.

 The point is to have the different objects/classes that have clear
 interaction/cooperation and otherwise hide the details of things that are
 unimportant to the caller.  The LogMap class 'knows' which log-lines it
 counts, DirectoryListing the wanted URLs etc.

 > Maybe I'm just hesitant, because I don't know yet how the future Metrics
 website will use databases for aggregation, what data format it will use
 for graph or table data, whether metrics-lib will support webstats logs,
 whether CollecTor will collect and serve webstats logs, and so on.  Any
 efforts we make in refactoring this code might be wasted if we make major
 changes in a few months.

 This effort will exactly pay off when these future changes are to be made.
 Then all code is in different class with a clear contract to the outside
 and all dirty implementation details hidden.  Modularization is about
 making future changes easier to accomplish.

 > Maybe we can combine discussing this specific code and thinking about
 the vision of a future Metrics website.  We can start doing that next week
 (better: next month), ideally after the Metrics and the Onionoo/Atlas
 deliverables are completed and invoiced.

 I didn't intend to start a lengthy re-coding discussion.  Just raising
 concerns when a new module is implemented in 'the old' way.
 We can just have a short session about these topics at the Berlin meeting,
 maybe?

 >
 > Regardless of the above, can you extract that bugfix you mentioned into
 a separate commit that I can cherry-pick?  Or can you describe it in more
 detail, so that I can fix it here?

 Compare [https://gitweb.torproject.org/karsten/metrics-
 web.git/tree/modules/webstats/src/main/java/org/torproject/metrics/webstats/Main.java?h=task-21236-4&id=9341942ebc2f62a3aa1fda7a255ffdeb9fbd7740#n203
 this line] with [https://gitweb.torproject.org/user/iwakeh/metrics-
 web.git/tree/modules/webstats/src/main/java/org/torproject/metrics/webstats/LogMap.java?h=task-21236-5&id=be64ba48fabd83b231646364beb5271c95900775#n56
 this], unfortunately I couldn't catch the log-line causing the error.
 {{{
 - if (!logLineMatcher.matches()) {
 + if (!logLineMatcher.matches() && logLineMatcher.groupCount() != 3) {
 }}}


 >
 > Note that there's also another trivial commit in
 [https://gitweb.torproject.org/karsten/metrics-web.git/log/?h=task-21236-4
 my task-21236-4 branch]:
 >
 >  - [https://gitweb.torproject.org/karsten/metrics-
 web.git/commit/?h=task-21236-4&id=9341942ebc2f62a3aa1fda7a255ffdeb9fbd7740
 9341942] fixes a syntax error in `metrics.json` that broke the webapp.
 >
 > Oh, the [https://metrics.torproject.org/webstats-tb.html Tor Browser
 downloads and updates" graph] is now deployed, but not yet linked from the
 homepage.  Please give it a try and see if anything is broken or unclear.
 >

 I'll poke at it now ;-)

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