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

Don Zickus dzickus at redhat.com
Tue Mar 26 13:36:58 UTC 2013


On Mon, Mar 25, 2013 at 05:46:34PM -0400, Cole Robinson wrote:
> > 
> > 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.

Ok.

> 
> > 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.

Ok.

> 
> >      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.

Yeah, I wasn't sure how you wanted this hooked in properly.

> 
> >      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.

Ok.

> 
> >      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.

Well the thing is this API is not consistent with the other calls which return
an array of bzs.  Here you have to de-reference the object r['bugs'] and
the returned object is not part of the _Bug object.  Which means
referencing the data beneath it is different than the other objects.

So I would argue to bit the bullet now while no one is using it and fix
the API to be consistent with the rest of the APIs.  In addition, a lot of
us don't use bin/bugzilla, instead we have our own scripts that reference
the python library and expect returned data to be in a certain format for
parsing.

Anyway, it's up to you.  Just wanted to point out my reason. :-)

Cheers,
Don


More information about the python-bugzilla mailing list