#34191: Combine multiple analysis files into single data set
-------------------------------+--------------------------------
Reporter: karsten | Owner: acute
Type: enhancement | Status: needs_revision
Priority: Medium | Milestone:
Component: Metrics/Onionperf | Version:
Severity: Normal | Resolution:
Keywords: | Actual Points: 0.5
Parent ID: #33321 | Points: 0.5
Reviewer: karsten | Sponsor: Sponsor59-must
-------------------------------+--------------------------------
Changes (by karsten):
* status: needs_review => needs_revision
Comment:
Thanks for working on this and providing a branch! I read your comment and
the code but did not test anything yet. Two concerns:
- I'm not sure if we can make it work with a variable number of arguments
to the `-d` parameter. The usage string is really misleading by saying `-d
PATH [LABEL ...]`, indicating that it accepts a single `PATH` and zero or
more `LABEL`s. We would want it to say `-d PATH [PATH ...] LABEL` there.
Do you know how to override that string? I didn't find something and
stopped looking at options that felt like hacking argparse.
- Regarding the limitation that the `LABEL` cannot be a path anymore, I
think that's problematic. For one, it's turning this change into a
backward incompatible one. And it's also not intuitive for new users who
don't know how the parameter works today. Imagine a case where somebody
puts all files for OnionPerf instance op-hk into one directory `op-hk/`
and all files for op-ab into another one `op-ab/`. If they want to
visualize these files using OnionPerf instance names as labels, they'd
either have to pick different labels or rename the directories. Doable,
but for sure surprising.
Maybe it's better to keep this simple by allowing just two arguments as we
do right now. Users will understand that they have to put all files for
one data set into a directory. No surprises, they'll get what they want.
What do you think?
Apart from these concerns about the user interface, the patch looks good
to me. The decisions about using `*json*` as pattern (you might want to
use `*onionperf.analysis.json*` here, now that I think about it) or using
functionality from the reprocessing module look good to me. I'd say if we
can figure out the potential user interface issues, this will be a quick
review. Thanks!
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/34191#comment:6>
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
