'Re: [python-bugzilla] [PATCH] Add is_logged_in utility method.'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       python-bugzilla
Subject:    Re: [python-bugzilla] [PATCH] Add is_logged_in utility method.
From:       Arun Babu Neelicattu <abn () redhat ! com>
Date:       2014-12-12 10:02:01
Message-ID: 885158317.9223057.1418378521188.JavaMail.zimbra () redhat ! com
[Download RAW message or body]

Thanks Cole, I can look at modifying the the token cache implementation sometime later. 

Patch submitted to list.

-arun

----- Original Message -----
> From: "Cole Robinson" <crobinso@redhat.com>
> To: "Arun Babu Neelicattu" <abn@redhat.com>
> Cc: python-bugzilla@lists.fedorahosted.org, pingou@pingoured.fr
> Sent: Thursday, December 11, 2014 11:22:19 AM
> Subject: Re: [python-bugzilla] [PATCH] Add is_logged_in utility method.
> 
> On 11/29/2014 10:46 PM, Arun Babu Neelicattu wrote:
> >
> >
> > ----- Original Message -----
> >> From: "Cole Robinson" <crobinso@redhat.com>
> >> To: abn@redhat.com, python-bugzilla@lists.fedorahosted.org
> >> Cc: pingou@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@redhat.com wrote:
> >>> From: Arun Babu Neelicattu <abn@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.
> >
> 
> Sorry for the delay. There's a new API User.valid_login queued for bugzilla
> 5:
> 
> http://bugzilla.readthedocs.org/en/latest/api/core/v1/user.html#valid-login
> 
> You pass it a login name and a token, it will tell you if the token is still
> valid for that user. We could use that to accomplish is_logged_in, but we'd
> have to extend our token cache to track the login account as well. Maybe just
> add a comment pointing at the above documentation and we can implement it
> later if we want.
> 
> >> - 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
> 
> Patch looks good to me. If you stick in the bit I mention above and send it
> here I'll apply it
> 
> - Cole
> 
> 
_______________________________________________
python-bugzilla mailing list
python-bugzilla@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/python-bugzilla

[prev in list] [next in list] [prev in thread] [next in thread] 