launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22061
[Merge] lp:~cjwatson/launchpad/upgrade-keystoneclient-swiftclient into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/upgrade-keystoneclient-swiftclient into lp:launchpad.
Commit message:
Upgrade to python-keystoneclient 0.7.1 and python-swiftclient 2.0.3.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/upgrade-keystoneclient-swiftclient/+merge/335391
These were the latest versions in Icehouse, to go with keystone 2014.1.5 and
swift 1.13.1 currently on the server. Upgrading to these allows us to use a
more recent version of pbr that is compatible with other dependencies we'd
like to upgrade.
We need a few tweaks to make everything work properly:
* quieten overly-noisy logging (needed until python-swiftclient 3.2.0);
* explicitly restrict HashStream to the desired segment size, since
otherwise the client will read past the end of the segment and we can end
up with corrupted hashes;
* advertise a keystone endpoint in the fake Swift fixture;
* cope with slightly different exception types due to the client now using
requests.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/upgrade-keystoneclient-swiftclient into lp:launchpad.
=== modified file 'constraints.txt'
--- constraints.txt 2017-12-19 09:46:14 +0000
+++ constraints.txt 2017-12-19 17:02:21 +0000
@@ -228,6 +228,7 @@
auditor==0.0.3
auditorclient==0.0.4
auditorfixture==0.0.7
+Babel==2.5.1
backports.lzma==0.0.3
BeautifulSoup==3.2.1
billiard==3.3.0.20
@@ -255,7 +256,7 @@
httplib2==0.8
importlib==1.0.2
ipython==0.13.2
-iso8601==0.1.4
+iso8601==0.1.12
jsautobuild==0.2
keyring==0.6.2
kombu==3.0.30
@@ -282,6 +283,7 @@
meliae==0.2.0.final.0
mock==1.0.1
mocker==1.1.1
+netaddr==0.7.19
oauth==1.0
oops==0.0.13
oops-amqp==0.0.8b1
@@ -290,9 +292,9 @@
oops-twisted==0.0.7
oops-wsgi==0.0.8
ordereddict==1.1
-oslo.config==1.1.1
+oslo.config==1.3.0
paramiko==1.7.7.2
-pbr==0.5.20
+pbr==0.11.1
pgbouncer==0.0.8
prettytable==0.7.2
psycopg2==2.6.1
@@ -305,14 +307,14 @@
pystache==0.5.3
python-dateutil==1.5
python-debian==0.1.23
-python-keystoneclient==0.3.1
+python-keystoneclient==0.7.1
python-memcached==1.58
python-mimeparse==0.1.4
# XXX: deryck 2012-08-10
# See lp:~deryck/python-openid/python-openid-fix1034376 which
# reapplied a patch from wgrant to get codehosting going again.
python-openid==2.2.5-fix1034376
-python-swiftclient==1.5.0
+python-swiftclient==2.0.3
PyYAML==3.10
rabbitfixture==0.3.6
requests==2.7.0
=== modified file 'lib/lp/services/librarianserver/librariangc.py'
--- lib/lp/services/librarianserver/librariangc.py 2017-04-02 01:17:50 +0000
+++ lib/lp/services/librarianserver/librariangc.py 2017-12-19 17:02:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Librarian garbage collection routines"""
@@ -55,7 +55,8 @@
swift_connection = swift.connection_pool.get()
container, name = swift.swift_location(content_id)
try:
- swift_connection.head_object(container, name)
+ with swift.disable_swiftclient_logging():
+ swift_connection.head_object(container, name)
return True
except swiftclient.ClientException as x:
if x.http_status != 404:
@@ -79,8 +80,9 @@
try:
swift_connection = swift.connection_pool.get()
container, name = swift.swift_location(content_id)
- chunks = swift_connection.get_object(
- container, name, resp_chunk_size=STREAM_CHUNK_SIZE)[1]
+ with swift.disable_swiftclient_logging():
+ chunks = swift_connection.get_object(
+ container, name, resp_chunk_size=STREAM_CHUNK_SIZE)[1]
return swift.SwiftStream(swift_connection, chunks)
except swiftclient.ClientException as x:
if x.http_status != 404:
@@ -562,7 +564,8 @@
container, name = swift.swift_location(content_id)
with swift.connection() as swift_connection:
try:
- swift_connection.delete_object(container, name)
+ with swift.disable_swiftclient_logging():
+ swift_connection.delete_object(container, name)
removed.append('Swift')
except swiftclient.ClientException as x:
if x.http_status != 404:
@@ -739,10 +742,11 @@
container_num += 1
container = swift.SWIFT_CONTAINER_PREFIX + str(container_num)
try:
- names = sorted(
- swift_connection.get_container(
- container, full_listing=True)[1],
- key=lambda x: map(int, x['name'].split('/')))
+ with swift.disable_swiftclient_logging():
+ names = sorted(
+ swift_connection.get_container(
+ container, full_listing=True)[1],
+ key=lambda x: map(int, x['name'].split('/')))
for name in names:
yield (container, name)
except swiftclient.ClientException as x:
=== modified file 'lib/lp/services/librarianserver/storage.py'
--- lib/lp/services/librarianserver/storage.py 2016-09-19 13:53:42 +0000
+++ lib/lp/services/librarianserver/storage.py 2017-12-19 17:02:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -10,9 +10,9 @@
import tempfile
from swiftclient import client as swiftclient
-from twisted.python import log
from twisted.internet import defer
from twisted.internet.threads import deferToThread
+from twisted.python import log
from twisted.web.static import StaticProducer
from lp.registry.model.product import Product
@@ -123,8 +123,12 @@
container, name = swift.swift_location(fileid)
swift_connection = swift.connection_pool.get()
try:
+ def get_object_quiet(*args, **kwargs):
+ with swift.disable_swiftclient_logging():
+ return swift_connection.get_object(*args, **kwargs)
+
headers, chunks = yield deferToThread(
- swift_connection.get_object,
+ get_object_quiet,
container, name, resp_chunk_size=self.CHUNK_SIZE)
swift_stream = TxSwiftStream(swift_connection, chunks)
defer.returnValue(swift_stream)
=== 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
@@ -1,12 +1,17 @@
-# Copyright 2013 Canonical Ltd. This software is licensed under the
+# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Move files from Librarian disk storage into Swift."""
__metaclass__ = type
__all__ = [
- 'to_swift', 'filesystem_path', 'swift_location',
- 'connection', 'connection_pool', 'SWIFT_CONTAINER_PREFIX',
+ 'SWIFT_CONTAINER_PREFIX',
+ 'connection',
+ 'connection_pool',
+ 'disable_swiftclient_logging',
+ 'filesystem_path',
+ 'swift_location',
+ 'to_swift',
]
from contextlib import contextmanager
@@ -29,6 +34,23 @@
ONE_DAY = 24 * 60 * 60
+@contextmanager
+def disable_swiftclient_logging():
+ # 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.
+ # 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.
@@ -118,7 +140,8 @@
container, obj_name = swift_location(lfc)
try:
- swift_connection.head_container(container)
+ with disable_swiftclient_logging():
+ swift_connection.head_container(container)
log.debug2('{0} container already exists'.format(container))
except swiftclient.ClientException as x:
if x.http_status != 404:
@@ -127,7 +150,8 @@
swift_connection.put_container(container)
try:
- headers = swift_connection.head_object(container, obj_name)
+ with disable_swiftclient_logging():
+ headers = swift_connection.head_object(container, obj_name)
log.debug(
"{0} already exists in Swift({1}, {2})".format(
lfc, container, obj_name))
@@ -188,7 +212,7 @@
assert segment <= 9999, 'Insane number of segments'
seg_name = '%s/%04d' % (obj_name, segment)
seg_size = min(fs_size - fs_file.tell(), MAX_SWIFT_OBJECT_SIZE)
- md5_stream = HashStream(fs_file)
+ md5_stream = HashStream(fs_file, length=seg_size)
swift_md5_hash = swift_connection.put_object(
container, seg_name, md5_stream, seg_size)
segment_md5_hash = md5_stream.hash.hexdigest()
@@ -304,13 +328,20 @@
class HashStream:
"""Read a file while calculating a checksum as we go."""
- def __init__(self, stream, hash_factory=hashlib.md5):
+ def __init__(self, stream, length=None, hash_factory=hashlib.md5):
self._stream = stream
+ self._length = self._remaining = length
self.hash_factory = hash_factory
self.hash = hash_factory()
def read(self, size=-1):
+ if self._remaining is not None:
+ if self._remaining <= 0:
+ return ''
+ size = min(size, self._remaining)
chunk = self._stream.read(size)
+ if self._remaining is not None:
+ self._remaining -= len(chunk)
self.hash.update(chunk)
return chunk
@@ -320,6 +351,8 @@
def seek(self, offset):
"""Seek to offset, and reset the hash."""
self.hash = self.hash_factory()
+ if self._remaining is not None:
+ self._remaining = self._length - offset
return self._stream.seek(offset)
=== modified file 'lib/lp/services/librarianserver/tests/test_gc.py'
--- lib/lp/services/librarianserver/tests/test_gc.py 2017-12-07 12:05:01 +0000
+++ lib/lp/services/librarianserver/tests/test_gc.py 2017-12-19 17:02:21 +0000
@@ -728,7 +728,8 @@
name += suffix
with swift.connection() as swift_connection:
try:
- swift_connection.head_object(container, name)
+ with swift.disable_swiftclient_logging():
+ swift_connection.head_object(container, name)
return True
except swiftclient.ClientException as x:
if x.http_status == 404:
=== modified file 'lib/lp/services/librarianserver/tests/test_swift.py'
--- lib/lp/services/librarianserver/tests/test_swift.py 2014-12-16 10:51:41 +0000
+++ lib/lp/services/librarianserver/tests/test_swift.py 2017-12-19 17:02:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd. This software is licensed under the
+# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Librarian disk to Swift storage tests."""
@@ -19,14 +19,17 @@
from lp.services.features.testing import FeatureFixture
from lp.services.librarian.client import LibrarianClient
from lp.services.librarian.model import LibraryFileAlias
+from lp.services.librarianserver import swift
from lp.services.librarianserver.storage import LibrarianStorage
from lp.services.log.logger import BufferLogger
from lp.testing import TestCase
-from lp.testing.layers import BaseLayer, LaunchpadZopelessLayer, LibrarianLayer
+from lp.testing.layers import (
+ BaseLayer,
+ LaunchpadZopelessLayer,
+ LibrarianLayer,
+ )
from lp.testing.swift.fixture import SwiftFixture
-from lp.services.librarianserver import swift
-
class TestFeedSwift(TestCase):
layer = LaunchpadZopelessLayer
@@ -293,7 +296,8 @@
def test_partial_read(self):
empty_sha1 = 'da39a3ee5e6b4b0d3255bfef95601890afd80709'
- s = swift.HashStream(StringIO('make me another coffee'), hashlib.sha1)
+ s = swift.HashStream(
+ StringIO('make me another coffee'), hash_factory=hashlib.sha1)
self.assertEqual(s.hash.hexdigest(), empty_sha1)
chunk = s.read(4)
self.assertEqual(chunk, 'make')
@@ -304,6 +308,21 @@
self.assertEqual(s.hash.hexdigest(),
'8c826e573016ce05f3968044f82507b46fd2aa93')
+ def test_limited_length(self):
+ base_stream = StringIO('make me a coffee')
+ s = swift.HashStream(base_stream, length=8)
+ chunk = s.read(4)
+ self.assertEqual(chunk, 'make')
+ self.assertEqual(s.hash.hexdigest(),
+ '099dafc678df7d266c25f95ccf6cde22')
+ chunk = s.read(8)
+ self.assertEqual(chunk, ' me ')
+ self.assertEqual(s.hash.hexdigest(),
+ '10a0334e435b75f35b1923842bd87f81')
+ self.assertEqual(s.read(), '')
+ self.assertEqual(s.tell(), 8)
+ self.assertEqual(base_stream.tell(), 8)
+
def test_tell(self):
s = swift.HashStream(StringIO('hurry up with that coffee'))
self.assertEqual(s.tell(), 0)
=== modified file 'lib/lp/testing/swift/fakeswift.py'
--- lib/lp/testing/swift/fakeswift.py 2017-12-07 12:05:13 +0000
+++ lib/lp/testing/swift/fakeswift.py 2017-12-19 17:02:21 +0000
@@ -557,17 +557,33 @@
"""Compute service catalog for the given request and tenant."""
port = request.transport.socket.getsockname()[1]
tenant_id = self.tenants[tenant]["id"]
- base_url = "http://%s:%d/swift/v1"; % (self.hostname, port)
+ base_url = "http://%s:%d"; % (self.hostname, port)
+ keystone_base_url = "%s/keystone/v2.0" % base_url
+ swift_base_url = "%s/swift/v1" % base_url
catalog = [
- {"endpoints": [
- {"adminURL": base_url,
- "id": uuid.uuid4().hex,
- "internalURL": base_url + "/AUTH_" + tenant_id,
- "publicURL": base_url + "/AUTH_" + tenant_id,
- "region": DEFAULT_REGION}
- ],
- "endpoints_links": [],
- "name": "swift",
- "type": "object-store"
- }]
+ {
+ "endpoints": [{
+ "adminURL": keystone_base_url,
+ "id": uuid.uuid4().hex,
+ "internalURL": keystone_base_url,
+ "publicURL": keystone_base_url,
+ "region": DEFAULT_REGION,
+ }],
+ "endpoints_links": [],
+ "name": "keystone",
+ "type": "identity",
+ },
+ {
+ "endpoints": [{
+ "adminURL": swift_base_url,
+ "id": uuid.uuid4().hex,
+ "internalURL": swift_base_url + "/AUTH_" + tenant_id,
+ "publicURL": swift_base_url + "/AUTH_" + tenant_id,
+ "region": DEFAULT_REGION,
+ }],
+ "endpoints_links": [],
+ "name": "swift",
+ "type": "object-store",
+ },
+ ]
return catalog
=== modified file 'lib/lp/testing/swift/tests/test_fixture.py'
--- lib/lp/testing/swift/tests/test_fixture.py 2017-12-07 12:05:13 +0000
+++ lib/lp/testing/swift/tests/test_fixture.py 2017-12-19 17:02:21 +0000
@@ -8,9 +8,10 @@
from datetime import datetime
from hashlib import md5
-import httplib
+from requests.exceptions import ConnectionError
from swiftclient import client as swiftclient
+from swiftclient.exceptions import ClientException
from testtools.matchers import (
GreaterThan,
LessThan,
@@ -200,14 +201,14 @@
# authenticated.
self.swift_fixture.shutdown()
self.assertRaises(
- httplib.HTTPException,
+ ConnectionError,
client.get_object, "size", str(size))
# And even if we bring it back up, existing connections
# continue to fail
self.swift_fixture.startup()
self.assertRaises(
- httplib.HTTPException,
+ ClientException,
client.get_object, "size", str(size))
# But fresh connections are fine.
Follow ups
