Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#2603 closed defect (fixed)

Remove dead parts from Torperf directory

Reported by: karsten Owned by: karsten
Priority: Medium Milestone:
Component: Metrics/CollecTor Version:
Severity: Keywords: TorPerfIteration20110305
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:

Description

The current Torperf directory contains at least two, maybe three files that can (and should) go away, because they blur the repository and confuse new users: README, run_test.py, and maybe plot_results.R. We should look if we still need something from these files and, if not, throw them away. While we're doing that, we could also update the LICENSE file.

I'm assigning this ticket to me. Will post a branch for others to review before merging.

Child Tickets

Change History (8)

comment:1 Changed 9 years ago by karsten

Keywords: TorPerfIteration20110305 added

comment:2 Changed 9 years ago by karsten

Status: newneeds_review

Please review branch task2603 in my public repository.

comment:3 Changed 9 years ago by Sebastian

Looks good mostly. In the README, maybe the link between the different files could be made clearer? extra_stats.py really is optional, too; and a note could be added that if it is used, consolidate_stats.py will help make sense of the output. On the other hand, maybe it's not that big a deal because people running torperf will already know what kind of experiment they want to do. hm.

comment:4 in reply to:  3 ; Changed 9 years ago by karsten

Replying to Sebastian:

Looks good mostly. In the README, maybe the link between the different files could be made clearer?

I think the real problem here is that we have too many independent scripts that may or may not be optional, depending on what you're doing. I think we should implement #2565 and get rid of most of the files described in the README. This cleanup is mostly for people looking into Torperf before that happens, but it's probably not perfect.

extra_stats.py really is optional, too; and a note could be added that if it is used, consolidate_stats.py will help make sense of the output.

extra_stats.py may be optional, but we included it in the basic setup instructions in measurements_HOWTO, so saying it's optional might confuse people. Also, consolidate_stats.py is broken right now, because we changed the .extradata file format (I just opened #2626 for that).

On the other hand, maybe it's not that big a deal because people running torperf will already know what kind of experiment they want to do. hm.

Right, whoever uses Torperf these days will have to know what they want to do. We should change that (#2565).

Sounds like we can merge?

comment:5 in reply to:  4 ; Changed 9 years ago by Sebastian

Replying to karsten:

Replying to Sebastian:

Looks good mostly. In the README, maybe the link between the different files could be made clearer?

I think the real problem here is that we have too many independent scripts that may or may not be optional, depending on what you're doing. I think we should implement #2565 and get rid of most of the files described in the README. This cleanup is mostly for people looking into Torperf before that happens, but it's probably not perfect.

Ah. I made everything into its own script so it would be easy to verify each on its own, and so that you only need to run those parts that you actually need. But if a single script does all that'd work too of course.

extra_stats.py really is optional, too; and a note could be added that if it is used, consolidate_stats.py will help make sense of the output.

extra_stats.py may be optional, but we included it in the basic setup instructions in measurements_HOWTO, so saying it's optional might confuse people. Also, consolidate_stats.py is broken right now, because we changed the .extradata file format (I just opened #2626 for that).

We should not break it next time before it is obsolete ;)

On the other hand, maybe it's not that big a deal because people running torperf will already know what kind of experiment they want to do. hm.

Right, whoever uses Torperf these days will have to know what they want to do. We should change that (#2565).

Hrm. Sounds like people should think about what they want to do before wasting bandwidth.

Sounds like we can merge?

Sure.

comment:6 in reply to:  5 Changed 9 years ago by karsten

Replying to Sebastian:

Ah. I made everything into its own script so it would be easy to verify each on its own, and so that you only need to run those parts that you actually need. But if a single script does all that'd work too of course.

You might want to look into #2565 and discuss how Torperf's UI should look like. My idea was to have a single config file and a main Python file to start a Torperf run. There might be better solutions, though.

We should not break it next time before it is obsolete ;)

Right. We figured the only person using it right now is Mike. Maybe we should do our next Torperf iteration in a separate branch that we merge into master once the iteration is over.

Sounds like we can merge?

Sure.

Great! Thanks for reviewing. Merging and closing.

comment:7 Changed 9 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Ermm, and now I'm closing the ticket for sure.

comment:8 Changed 9 years ago by dchasteen

Points: 1

Added points from sprint planning meeting.

Note: See TracTickets for help on using tickets.