715884 - replace optparse usage with argparse

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Burak Yiğit Kaya [:BYK]]

MozHttpd uses the old deprecated optparse module where it should use arparse for future-proof code. Also I noticed it fails to run when you try to use it without any arguments(it especially fails on "test"). Although this should be a separate bug, I hope to solve it with this transition.

Comment 1

[Burak Yiğit Kaya [:BYK]]

Attachment: Improves the CLI options via argparse module

Comment 2

[William Lachance (:wlach)]

(In reply to Burak Yiğit Kaya [:BYK] from comment #0)
> MozHttpd uses the old deprecated optparse module where it should use arparse
> for future-proof code. Also I noticed it fails to run when you try to use it
> without any arguments(it especially fails on "test"). Although this should
> be a separate bug, I hope to solve it with this transition.

I agree that argparse has a nicer API than optparse, but I don't think we can use argparse until we upgrade to Python 2.7 across our automation (yes there are hacks possible like including the argparse.py inside mozhttpd but I don't think we want to go there). For now, I think the easiest route is to keep optparse and deal with its imperfections.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

I like the patch and it looks good.  I don't think there is an easy way to do a :

try:
  import argparse
except:
  import optparse

that would be too hacky.  It is too premature to split our tools to have two versions (older python and python2.7).  

I believe we could achieve our python 2.7 goal must faster than end of year, and if so we should keep this patch around and land it then.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 586426 [details] [diff] [review]
Improves the CLI options via argparse module

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

adding r- for now until we get out of python 2.4.

Comment 5

[Jeff Hammel]

I am guessing this actually affects all of mozbase.  Is that correct? (Well, all CLI parsers therein anyway).  But yes, this is blocked on requiring python 2.7 for mozbase which is blocked by having python 2.7 on all test machines: bug 711299 (and probably all build machines too: bug 602908).

Comment 6

[Burak Yiğit Kaya [:BYK]]

How can it affect the whole mozbase? I mean the patch itself cannot affect anything else tho if you would like to use argparse for everything, that would be great but a big undertaking. Also it may need unbitrotting.

Comment 7

[Jeff Hammel]

So we're not going to be requiring python 2.7 any time soon (end of year is a best-case scenario).   So my question is....when we do require python 2.7, do we want to move all of mozbase to arpparse?

(And yes, the patch is bitrotted anyway.)

Comment 8

[Burak Yiğit Kaya [:BYK]]

I would say yes since optparse is ancient and deprecated AFAIK.

Comment 9

[(not currently active) Ted Mielczarek]

jmaher claims we got Talos onto Python 2.7, so this should be fixable now. Resummarizing, we should just do this wholesale.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

ah, but foopies run 2.6.6, that would affect android tests (I just learned this last night)

Comment 11

[Justin Wood (:Callek)]

(In reply to Joel Maher (:jmaher) from comment #10)
> ah, but foopies run 2.6.6, that would affect android tests (I just learned
> this last night)

This no longer matters (we're no longer using foopies) so this can proceed I think.

Comment 12

[Edwin Takahashi (:egao | infrequent contributor)]

Attachment: Bug 715884 - replace optparse usage with argparse r?jmaher

- changed argument parser to use ArgumentParser in compliance with Python 2.7.

Comment 13

[Edwin Takahashi (:egao | infrequent contributor)]

A (what I think is) fairly extensive try run:

https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=superseded,testfailed,busted,exception,runnable&revision=aed7808a601a8cb253a5d333d0e09edcce4a9979&selectedJob=200543609

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 9010788 [details]
Bug 715884 - replace optparse usage with argparse r?jmaher

Joel Maher ( :jmaher ) (UTC-4) has approved the revision.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e81a2482bed6
replace optparse usage with argparse r=jmaher

Comment 16

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/e81a2482bed6