Closed Bug 1059872 Opened 10 years ago Closed 10 years ago

mozregression should have a --version option

Categories

(Testing :: mozregression, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wlach, Assigned: devty1023, Mentored)

Details

(Whiteboard: [good first bug][lang=python])

Attachments

(1 file)

For troubleshooting what version of mozregression someone has, it would be useful to have a '--version' switch to the command line client which prints this to standard out.

Prerequisites:

Basic knowledge of python, virtualenvs.

Procedure to do this:

1. Clone mozregression source (http://github.com/mozilla/mozregression), create a virtualenv and install it inside it

2. Modify mozregression's __init__.py to contain a single string called "__version__ = "0.23"

3. Modify setup.py to include:

from mozregression import __version__

Then specify version=__version__ inside the setup clause

4. Finally, modify mozregression's cli (mozregression/regression.py) to include a "--version" option which, if set to true, will cause the __version__ string to be printed out and the program to exit.
Why you won't like to do it like mozbase packages are handling that? The above solution might be error prone, so when the version gets bumped, __init__.py might be forgotten.

Here the mozbase solution:
http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/cli.py#66
(In reply to Henrik Skupin (:whimboo) from comment #1)
> Why you won't like to do it like mozbase packages are handling that? The
> above solution might be error prone, so when the version gets bumped,
> __init__.py might be forgotten.
> 
> Here the mozbase solution:
> http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/
> mozrunner/cli.py#66

Well, with above approach setup.py would get the version from inside mozregression, so you wouldn't have that problem. I'd be fine with the above solution though, I guess it would even be nicer in the sense that you would still bump versions by modifying setup.py.
I will like to challenge myself with this as my first bug fix. I will follow the directions provided above and will (probably) ask for further guidance as I try to complete the ticket.
I've implemented the required code, and I need guidance taking the next step (submitting the code for review, checking it in, etc.). 

Would really appreciate anyone's help.
(In reply to Daniel Y Lee from comment #4)
> I've implemented the required code, and I need guidance taking the next step
> (submitting the code for review, checking it in, etc.). 
> 
> Would really appreciate anyone's help.

Hi Daniel,

We usually do pull-requests via github. Follow this: http://globau.wordpress.com/2013/10/21/github-pull-requests-and-bugzilla/ for adding the PR to bugzilla and mark the "review" field with a ? for either me or William. We will review the PR and provide any feedback.

Thanks!
Attachment #8480950 - Flags: review?(wlachance)
Attachment #8480950 - Flags: review?(samdgarrett)
Thank you for your kind reply. Looking forward to the review!
Comment on attachment 8480950 [details] [review]
Bug 1059872 - mozregression should have a --version option

Hey, so the patch is pretty much exactly what I asked before, but after some consideration I really think we should use the approach that whimboo suggested in comment 1, as that will (1) keep us consistent with other python software at mozilla and (2) match people's default intuition that setup.py is the place to update the version.

Thanks for your effort so far! Looking forward to an update.
Attachment #8480950 - Flags: review?(wlachance) → review-
sure thing. I will consider the example provided in comment 1 and will implement the code tonight!
Hi,

I'm having a bit of problem following the example mentioned in comment 1 and am seeking help!

For reference, I'm reading through this code:

http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/cli.py#66

In particular:

66         self.metadata = getattr(sys.modules[self.module],
67                                 'package_metadata',
68                                 {})
69         version = self.metadata.get('Version')

I have been unable to locate where in the mozrunner code the "package_metadata" is set for the above module. My best guess is that "def get_metadata_from_egg(self):" (http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/cli.py#118) is being called somewhere to load the package metadata, but I couldn't find a single invoation of the above function anywhere in the code repository.

My current plan is to copy over the portion of code defined under "get_metadata_from_egg()" function directly to the mozregression's cli code. I don't see this as a particularly efficient implementation (for instance, import pkg_resources will be called every time version number is requested), but still a workable solution.

In summary:

Please help me understand how mozrunner is getting metadata in setup.py. I suspect "get_metadata_from_egg()" is involved, but not entirely sure how it is being used to set 'package_metadata' attribute for the module.

Thanks
Hi Daniel, so I looked into this in a bit more detail and it's not clear to me how the code in mozrunner works either. Running 'mozrunner --version' certainly doesn't seem to work. I think it's also worth noting that none of the advice on this stackoverflow question recommends anything like that:

http://stackoverflow.com/questions/458550/standard-way-to-embed-version-into-python-package

Let's just fall back to your original approach. It's simple and works great! If Henrik or anyone else has a proposal for improving things further here, we can always followup in a seperate patch.
Comment on attachment 8480950 [details] [review]
Bug 1059872 - mozregression should have a --version option

Changing to r+. I'll cancel Sam's review, since we really only need one.
Attachment #8480950 - Flags: review?(samdgarrett)
Attachment #8480950 - Flags: review-
Attachment #8480950 - Flags: review+
https://github.com/mozilla/mozregression/commit/3664ba1f642168defc2059a18760f057dccbef84
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Filed bug 1060940 to try and sort out what's going on with mozrunner.
Assignee: nobody → devty1023
Very cool! Thanks for your tips and advices.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: