[python-bugzilla] [PATCH] Add is_logged_in utility method.

Arun Babu Neelicattu abn at redhat.com
Sun Nov 30 03:46:55 UTC 2014



----- Original Message -----
> From: "Cole Robinson" <crobinso at redhat.com>
> To: abn at redhat.com, python-bugzilla at lists.fedorahosted.org
> Cc: pingou at pingoured.fr
> Sent: Sunday, November 30, 2014 5:46:19 AM
> Subject: Re: [python-bugzilla] [PATCH] Add is_logged_in utility method.
> 
> On 11/24/2014 06:42 PM, abn at redhat.com wrote:
> > From: Arun Babu Neelicattu <abn at redhat.com>
> >
> > ---
> >   bugzilla/base.py | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> >
> > diff --git a/bugzilla/base.py b/bugzilla/base.py
> > index e06223f..7aba942 100644
> > --- a/bugzilla/base.py
> > +++ b/bugzilla/base.py
> > @@ -697,6 +697,26 @@ class BugzillaBase(object):
> >           self.password = ''
> >           self.logged_in = False
> >
> > +    def is_logged_in(self):
> > +        """
> > +        Utility method to check if this instance has already been logged
> > in.
> > +
> > +        If the instance has its logged_in attribute set to True, this
> > method
> > +        returns true. Otherwise, to test if this session is authenticated,
> > the
> > +        method calls the User.get() XMLRPC method with ids set. Logged-out
> > users
> > +        cannot pass the 'ids' parameter and will result in a 505 error.
> > +        """
> > +        try:
> > +            if self.logged_in:
> > +                return True
> > +            self._proxy.User.get({'ids': self._listify([])})
> 
> listify is redundant here since we know we are passing a list. (listify is
> just a helper to turn None into a list, where it's required)

doh! ack

> 
> > +            return True
> > +        except Fault:
> > +            e = sys.exc_info()[1]
> > +            if e.faultCode == 505:
> > +                return False
> > +            raise e
> > +
> >
> >       #############################################
> >       # Fetching info about the bugzilla instance #
> >
> 
> Having this capability is definitely useful. But a few ideas:
> 
> - We should file a bug with upstream bugzilla to actually provide a real API
> for this. Bugzilla 5.0 may actually have an API for this... I recall some
> change WRT login state, but I'm not certain. I can check next week
> 

Agreed, could not find any reference to the 5.0 change though, will wait before filing anything.

> - I think there's places in the test suite where we already try to
> approximate
> this, grep _check_rh_privs. Might be able to use this as well. It might not
> fit though, but regardless I'd like to have some sort of unit test for this
> 

I have added a proper unit test for this test10LoginState. If we merge these changed we can probably get rid of _check_rh_privs I suppose.

> - After this we will have an API disparity: there's self.logged_in which
> still
> has confusing semantics as pingou pointed out. So maybe turn self.logged_in
> into a readonly property that will call this function, and cache the value.
> Dunno if it's worth it
> 

This makes sense, I have dropped the caching since i reckon it is rather pointless (probably even less correct?). I am assuming users will only check probably only one at the beginning of the call and not before every bz.* call. If the latter is the case we can easily add caching by setting self._logged_in. For now I have made self.logged_in make the Users.get call every time.

See proposed changes and tests at https://github.com/abn/python-bugzilla/commit/bff0df85690d787b7560e5a88a0c9804146f8d94

Will wait before I send the new patch, feel free to leave comments there.

-arun

> - Cole
>


More information about the python-bugzilla mailing list