#22112: Replace torperf.csv with onionperf.csv -------------------------------------+------------------------------ Reporter: karsten | Owner: metrics-team Type: enhancement | Status: needs_review Priority: Medium | Milestone: Component: Metrics/Metrics website | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: | Sponsor: -------------------------------------+------------------------------ Changes (by karsten):
* status: needs_revision => needs_review Comment: Replying to [comment:2 iwakeh]: > Committed to master means 'productive', but the current onionperf.csv links on metrics.tp.o lead to 404; is that intended? Ah, no, that's an oversight. Fixed! (Just not yet deployed.) > init-onionperf.sql: The last "group by" on the second to the last line contains a `3`, which seems to refer to `source`, but `source` in this select is set to `''`. What is intended here? The intention was to aggregate over all sources and to include `''` in every resulting row, so that there's the same number and type of columns as in the `SELECT` above. That third column, `'' AS source`, is not an aggregate function, so I included it in the `GROUP BY`. But I just tried out to simply drop that column from the `GROUP BY`, which worked just fine. Changed. > Why are there columns in the measurements table that are not used later on (mostly the unrecognized key lines))? Shouldn't these be omitted? > Here it seems as if only 'endpointremote' is used to determine the content of column 'server'. If just that is needed it ought to be filled by the java code initially. But, I might have overlooked something? I tried to make the schema as complete as possible, so that we can write different views in the future more easily. In fact, I only left out path information and build times, because I wanted to avoid making the schema too complex while not using the data at all. But I could see how we're adding those parts, too. Regarding your question why we included unrecognized keys, they will all be recognized in the next metrics-lib version, in which case we'll update this code. > In some other module we used constants for column-names in Java. As the db redesign will be future work, is this intentionally left to be improved then? Well, I'm not a big fan of how we used string constants there, because they give a false sense of code robustness by protecting against typos, but they cannot be used in prepared statements and also don't provide any type safety. And at least during development I ran much more often into the latter two issues than into typos. But maybe you're right and string constants are better than nothing. Changed. Please find [https://gitweb.torproject.org/karsten/metrics- web.git/log/?h=task-22112 my task-22112 branch] with the fixes. What do you think? Ready to remove the beta label and call this done? :) -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22112#comment:3> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs