launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code

We'll probably need to tweak staging configs a bit to effectively test this. Have you verified in any depth against a non-prod Swift?

Diff comments:

> 
> === modified file 'lib/lp/services/librarianserver/swift.py'
> --- lib/lp/services/librarianserver/swift.py	2015-02-16 00:09:14 +0000
> +++ lib/lp/services/librarianserver/swift.py	2017-12-19 17:02:21 +0000
> @@ -29,6 +34,23 @@
>  ONE_DAY = 24 * 60 * 60
>  
>  
> +@contextmanager
> +def disable_swiftclient_logging():

Given how this tends to be used, I'd be tempted to make it dammit_openstack_first_pbr_now_this(func, *args, **kwargs), allowing you to collapse most of the wrapper functions. A few extra assignment operations (due to the lack of nesting) won't hurt that much.

> +    # swiftclient has some very rude logging practices: the low-level API
> +    # calls `logger.exception` when a request fails, without considering
> +    # whether the caller might handle it and recover.  This was introduced
> +    # in 1.6.0 and removed in 3.2.0; until we're on a new enough version not
> +    # to need to worry about this, we shut up the noisy logging around calls
> +    # whose failure we can handle.

Do XXX this; we'll upgrade past the badness relatively soon, but this unhighlighted comment is sure to be missed.

> +    # Messier still, logging.getLogger('swiftclient') doesn't necessarily
> +    # refer to the Logger instance actually being used by swiftclient, so we
> +    # have to use swiftclient.logger directly.
> +    old_disabled = swiftclient.logger.disabled
> +    swiftclient.logger.disabled = True
> +    yield
> +    swiftclient.logger.disabled = old_disabled
> +
> +
>  def to_swift(log, start_lfc_id=None, end_lfc_id=None, remove_func=False):
>      '''Copy a range of Librarian files from disk into Swift.
>  


-- 
https://code.launchpad.net/~cjwatson/launchpad/upgrade-keystoneclient-swiftclient/+merge/335391
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.