Joel Maher ( :jmaher ) (UTC -8) Reporter
	Description • 10 years ago

modernizing the code base of talos, we should start with the commandline parser.

	Ujjwal Wahi [:ujjwal] Assignee
	Comment 1 • 10 years ago

I would like to work on this.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 2 • 10 years ago

in talos, the main cli stuff we need to worry about is in configuration.py, we define our arguments here:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l32

In fact, I do wonder if we could just get rid of configuration.py.

Talos has a variety of commandline options and we also write out a yaml file with the config so we could theoretically run the test again.  We used to have a need for input from .config files such that we could configure the defaults that way.  That is not a requirement anymore which makes our needs much simpler.

We have a few other cli usages in Talos, this bug should be specific to PerfConfigurator and ensuring all the cli options are represented properly.

Once this is done we can sort out compare.py, pageloader, and xperf related scripts.

	Ujjwal Wahi [:ujjwal] Assignee
	Comment 3 • 10 years ago

Facing problem, there is a class ConfigurationOption in configuaration.py which inherits `optparse.Option` and override `take_action` method of `optparse.Option`, Having problem figuring out what will be its equivalent in argparse.

Another difficulty is in `parse_args` method of Configuration class, I updated the code as per upgrade guidelines here https://argparse.googlecode.com/svn/trunk/doc/argparse-vs-optparse.html, but its breaking rest of the code in the method

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 4 • 10 years ago

Glad to see you are making progress here.  I think the configuration class might not be needed as much, maybe PerfConfigurator could subclass argparse and implement whatever output function is needed?

I will have to dig into the code in depth a bit more to help with a specific problem.  Just keep in mind that you can feel free to completely remove configuration.py or rework it however you need it to be.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 5 • 9 years ago

Hi Ujjwal, checking in with you here.  Do you need some help or have any questions?

	Ujjwal Wahi [:ujjwal] Assignee
	Comment 6 • 9 years ago

Hi Joel, currently busy with my exams. Was working on this issue before exams. There was little problem in starting regarding optparse and argparse, but now I am comfortable with them. Sorry for delay. Will resume on this after 31st Jan. 

Just one doubt, there is support for YAML and JSON configuration in http://hg.mozilla.org/build/talos/file/4693475155fa/talos/configuration.py#l82. Is it still required there?

Will try to create a patch as soon as possible. Thank You.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 7 • 9 years ago

we store the output in a .yml format, that was there so we could easily edit it.  I would like to keep it, but if we can simplify a lot of code, I could be fine with a raw .json format for storing data!

	Ujjwal Wahi [:ujjwal] Assignee
	Comment 8 • 9 years ago

Replaces optparse with argparse

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 9 • 9 years ago

Comment on attachment 8566057 [details] [diff] [review]
bug-1117190-argparse.diff

Review of attachment 8566057 [details] [diff] [review]:
-----------------------------------------------------------------

hey, this is nice and simple, let me run this on try server to see if anything odd comes up.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 10 • 9 years ago

testing here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab106dd67dde

there are a few other random changes, so non green might not be a big concern.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 12 • 9 years ago

Comment on attachment 8566656 [details] [diff] [review]
bug-1117190-argparse.diff

Review of attachment 8566656 [details] [diff] [review]:
-----------------------------------------------------------------

awesome, thanks for the quick fix!

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 13 • 9 years ago

https://hg.mozilla.org/build/talos/rev/9b8ddbb7fb27