Closed Bug 715884 Opened 8 years ago Closed 11 months ago

replace optparse usage with argparse

Categories

(Testing :: Mozbase, defect)

defect
Not set

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: BYK, Assigned: BYK)

References

Details

Attachments

(2 files)

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.
Assignee: nobody → madbyk
Status: NEW → ASSIGNED
(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.
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 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.
Attachment #586426 - Flags: review?(jmaher) → review-
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).
Blocks: 71299
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.
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.)
I would say yes since optparse is ancient and deprecated AFAIK.
jmaher claims we got Talos onto Python 2.7, so this should be fixable now. Resummarizing, we should just do this wholesale.
Summary: Mozhttpd should use the new standard argparse module for cli → replace optparse usage with argparse
ah, but foopies run 2.6.6, that would affect android tests (I just learned this last night)
(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.
- changed argument parser to use ArgumentParser in compliance with Python 2.7.
Comment on attachment 9010788 [details]
Bug 715884 - replace optparse usage with argparse r?jmaher

Joel Maher ( :jmaher ) (UTC-4) has approved the revision.
Attachment #9010788 - Flags: review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e81a2482bed6
replace optparse usage with argparse r=jmaher
https://hg.mozilla.org/mozilla-central/rev/e81a2482bed6
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.