[PATCH] Make the Welcome spoke wait for Geolocation lookup to finish (#975193)

Martin Kolman mkolman at redhat.com
Thu Jul 4 11:25:33 UTC 2013


On Mon, 2013-07-01 at 15:12 +0200, Vratislav Podzimek wrote:
> > +def _get_location_info_instance(wait=False):
> > +    """Just return instance of the location info object
> > +    and optionally wait for the Geolocation thread to finish
> > +
> > +    Meaning of the wait parameter:
> > +    - False or <=0: don't wait
> > +    - True : wait for default number of seconds specified by
> > +    the GEOLOC_TIMEOUT constant
> > +    - >0 - wait for a given number of seconds
> > +    :param wait: specifies if this function should wait
> > +    for the lookup to finish before returning the instance
> > +    :type wait: bool or integer
> > +
> > +    If there is no lookup in progress (no Geolocation refresh thread
> > +    is running, this function returns at once).
> > +    """
> > +
> > +    if wait:
> > +        # check if wait is boolean or a number
> > +        if wait is True:
> > +            wait = constants.GEOLOC_TIMEOUT
> > +        time_elapsed = 0
> > +        # first check if the geolocation thread is running
> > +        if threadMgr.get(constants.THREAD_GEOLOCATION_REFRESH):
> > +            while time_elapsed <= wait:
> > +                # refresh still in progress
> > +                if threadMgr.get(constants.THREAD_GEOLOCATION_REFRESH):
> > +                    time.sleep(GEOLOC_TIMEOUT_CHECK_INTERVAL)
> > +                    time_elapsed += GEOLOC_TIMEOUT_CHECK_INTERVAL
> > +                else:
> > +                    break
> > +            else:  # timed out
> > +                log.info("Waiting for Geolocation timed out after %d seconds."
> > +                         % wait)
> > +                # please note that this does not mean that the actual
> > +                # geolocation lookup was stopped in any way, this will
> > +                # just unblock the caller after the given wait period
> > +                # even though the lookup thread is still running
> > +    return location_info_instance
> > +
> I'd suggest two changes here:
> 1) invert the if, so that it returns immediately if bool(wait) is False
> and otherwise tries wait. That would decrease the indentation and
> clarify the code (at least for me) and 
Sure, why not. :)

> 
> 2) I think it might be better to use Queue.Queue and its 'get' method
> with a timeout specified. It perfectly fits this case and it cannot be
> worse than busy waiting implemented on our own. ;)
I've actually converted it to use the Python Condition[1] lock object:
* the caller will check if a refresh is in progress and wait for the
condition with a timeout
* once the refresh is done, the refresh thread will notify all threads
waiting for the condition (which will unblock them)

Gets rid of the busy waiting and IMHO also looks quite a bit cleaner.

[1] http://docs.python.org/2/library/threading.html#condition-objects
> 
> Otherwise this looks good to me. I believe it shouldn't cause any
> noticeable delay.
> 
I'll send a V2, but I'd like to test it first and I'm having some issues
with the current master crashing before it gets to the welcome
screen. :)



More information about the anaconda-patches mailing list