----- Original Message -----
From: "Cole Robinson" <crobinso(a)redhat.com>
To: abn(a)redhat.com, python-bugzilla(a)lists.fedorahosted.org
Cc: pingou(a)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(a)redhat.com wrote:
> From: Arun Babu Neelicattu <abn(a)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/bff0df85690d787b7560e5a88a0...
Will wait before I send the new patch, feel free to leave comments there.
-arun
- Cole