[python-bugzilla] Using history from the CLI (per https://lists.fedorahosted.org/pipermail/python-bugzilla/2013-February/000040.html)

Cole Robinson crobinso at redhat.com
Mon Mar 25 21:46:34 UTC 2013


On 03/25/2013 04:57 PM, Don Zickus wrote:
> On Mon, Mar 25, 2013 at 04:37:14PM -0400, Cole Robinson wrote:
>> On 03/22/2013 03:38 PM, Don Zickus wrote:
>>> On Mon, Mar 18, 2013 at 10:37:01PM +0200, Yaniv Kaul wrote:
>>>> Is the history exposed from the CLI? Looks like I need to use python
>>>> to access it?
>>>
>>> Hi,
>>>
>>> I implemented this privately awhile ago.  Took some time to port to 0.8.0,
>>> but you can try this patch to see if it is what you are looking for.  The
>>> output could use suggestions.
>>>
>>
>>
>> Cool, thanks for this Don! Though this patch doesn't apply to 0.8.0 for me...
>> did you send the wrong one? Or did I misunderstand and this patch still needs
>> to be ported to 0.8.0?
> 
> Grr. I suck.  I sent my original patch from last year.  Here is the one
> that I tested on top of 0.8.0 from last week. My apologies for that.
> 
> Cheers,
> Don
> 
> ---------------8<------------
> 
> From: Don Zickus <dzickus at redhat.com>
> Date: Fri, 22 Mar 2013 15:19:02 -0400
> Subject: [PATCH] bugzilla: New command 'history'
> 
> This patch just adds supports for reviewing the history of bugzillas.
> 
> I hacked up most of this patch a while ago for some other reason.  I spent an
> hour this morning migrating it to 0.8.0.
> 
> The data is valid, but the output display could probably use some suggestions.
> For now I stuck with something that mimic'd what the web GUI would show.
> 
> Most of the work is done on the front end bugzilla CLI.  A couple of output
> format hacks added because I didn't know the right way to hook things in.
> 
> The backend has a little tweak because the data returned from the server needs
> to be dereferenced correctly (for b in r['bugs']) like every other RPC call to
> the server.
> 
> Signed-off-by: Don Zickus <dzickus at redhat.com>
> ---
>  bin/bugzilla     |   43 +++++++++++++++++++++++++++++++++++++++++--
>  bugzilla/base.py |    3 ++-
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 

We'll also need a change to the docs, there's a manual bit in bin/bugzilla
which enumerates the commands for the man page.

> diff --git a/bin/bugzilla b/bin/bugzilla
> index 881c44e..67c8960 100755
> --- a/bin/bugzilla
> +++ b/bin/bugzilla
> @@ -27,7 +27,7 @@ import bugzilla
>  default_bz = 'https://bugzilla.redhat.com/xmlrpc.cgi'
>  
>  _is_unittest = bool(os.getenv("__BUGZILLA_UNITTEST"))
> -cmdlist = ['login', 'new', 'query', 'modify', 'attach', 'info']
> +cmdlist = ['login', 'new', 'query', 'modify', 'attach', 'info', 'history']
>  format_field_re = re.compile("%{([a-z0-9_]+)(?::([^}]*))?}")
>  
>  log = bugzilla.log
> @@ -309,6 +309,10 @@ def setup_action_parser(action):
>          p.set_usage('%prog login [username [password]]')
>          p.set_description("Log into bugzilla and save a login cookie.")
>  
> +    elif action == 'history':
> +        p.set_usage('%prog history BUGID [BUGID...]')
> +        p.set_description("Prints out the change history of the bugzillas")
> +

I'd also add an explicit disclaimer to the description that the output of this
command should not be relied on, and if you need to parse this information,
use the API directly. That way if we come up with a nicer way to format in the
future we are covered.

>      if action in ['new', 'query']:
>          outg = optparse.OptionGroup(p, "Output format options")
>          outg.add_option('-f', '--full', action='store_const', dest='output',
> @@ -619,6 +623,11 @@ def _convert_to_outputformat(output):
>          fmt += "#%{bug_id} %{status} %{assigned_to} %{component}\t"
>          fmt += "[%{target_milestone}] %{flags} %{cve}"
>  
> +    elif output == 'history':
> +        fmt += "Bugzilla %{bug_id}\n"
> +        fmt += "%-20s %-20s %-20s %-20s %-20s\n" % ("Who", "When", "What", "Removed", "Added")
> +        fmt += "%{history}"
> +

Hmm, I think cramming this in here is weird. This function is meant to convert
the user passed --outputformat convenience values. 'history' isn't one of
them, it's really an internal detail.

>      else:
>          raise RuntimeError("Unknown output type '%s'" % opt.output)
>  
> @@ -674,6 +683,20 @@ def _format_output(bz, opt, buglist):
>                  val += ("\n* %s - %s:\n%s\n" %
>                          (c['time'], c['author'], c['text']))
>  
> +        elif fieldname == "history":
> +            val = ""
> +            for h in getattr(b, "history", []):
> +                who = h['who']
> +                when = h['when']
> +                changes = h['changes']
> +                for c in changes:
> +                    val += ("%-20s %-20s %-20s %-20s %-20s\n\n" % (who, when,
> +                            c['field_name'], c['removed'], c['added']))
> +
> +                    #make it obvious when multiple changes happened at once
> +                    who = ""
> +                    when = ""
> +
>          elif fieldname == "__unicode__":
>              val = unicode(b)
>          else:
> @@ -845,6 +868,11 @@ def _do_set_attach(bz, opt, parser, args):
>          attid = bz.attachfile(bugid, fileobj, desc, **kwargs)
>          print "Created attachment %i on bug %s" % (attid, bugid)
>  
> +def _do_history(bz, opt, args):
> +    bugid_list = reduce(lambda l1, l2: l1 + l2,
> +                        [a.split(",") for a in args])
> +
> +    return bz.bugs_history(bugid_list)
>  
>  #################
>  # Main function #
> @@ -992,11 +1020,22 @@ def main(bzinstance=None):
>  
>          _do_modify(bz, opt, args)
>  
> +    elif action == 'history':
> +        if not args:
> +            parser.error('No bug IDs given '
> +                         '(maybe you forgot an argument somewhere?)')
> +
> +        #opt hacks
> +        opt.output = ""
> +        opt.outputformat = _convert_to_outputformat('history')
> +        newbugs = _do_history(bz, opt, args)
> +        buglist += newbugs
> +

Rather than hijack outputformat, I'd just opencode this as part of
_do_history. Then we also have more control over the format, say we wanted to
have variable sized columns based on the largest value for any given column.

>      else:
>          raise RuntimeError("Unexpected action '%s'" % action)
>  
>      # If we're doing new/query/modify, output our results
> -    if action in ['new', 'query']:
> +    if action in ['new', 'query', 'history']:
>          _format_output(bz, opt, buglist)
>  
>  
> diff --git a/bugzilla/base.py b/bugzilla/base.py
> index c2b9295..31b668c 100644
> --- a/bugzilla/base.py
> +++ b/bugzilla/base.py
> @@ -830,7 +830,8 @@ class BugzillaBase(object):
>          Experimental. Gets the history of changes for
>          particular bugs in the database.
>          '''
> -        return self._proxy.Bug.history({'ids': bug_ids})
> +        r = self._proxy.Bug.history({'ids': bug_ids})
> +        return [_Bug(bugzilla=self, dict=b) for b in r['bugs']]
>  

Hmm, I can understand the appeal here, but if the changes I suggest are made,
this isn't needed any more to do bin/bugzilla outputting, and I'd rather not
break API if we don't have to. Granted this bit was only added last month,
and there probably aren't many users yet, but it's in 0.8.0.

Also, I understand that you were just posting this patch in a reply and may
not have time or inclination to address review comments, so no pressure.

Thanks,
Cole


More information about the python-bugzilla mailing list