Rev 2748: * Move transport logging into a new transport class in http://people.ubuntu.com/~robertc/baz2.0/transport
Robert Collins
robertc at robertcollins.net
Tue Oct 2 06:43:07 BST 2007
At http://people.ubuntu.com/~robertc/baz2.0/transport
------------------------------------------------------------
revno: 2748
revision-id: robertc at robertcollins.net-20071002053339-vnyjf4jrxv0jeekf
parent: robertc at robertcollins.net-20070827041949-br263tkuxayldxoc
committer: Robert Collins <robertc at robertcollins.net>
branch nick: transport-get-file
timestamp: Tue 2007-10-02 15:33:39 +1000
message:
* Move transport logging into a new transport class
TransportTraceDecorator (trace+ to get it from a url).
* Give Registry a useful __repr__.
* Fix a bug introduced by the change to use a Registry for transport where
the transport loading tests left global state behind due to the
_get_protocol_handlers method returning a reference, not a value object.
* Add an upper_limit parameter to readv, because when asking for byte
ranges within the latency-adjustment window near the end of the file
causes errors that are tricky to manage.
* A few minor drive-by formatting fixes.
* The TransportDecorator constructor now accepts a _from_transport
parameter for decorators that want to share state (used by the trace
decorator)
added:
bzrlib/transport/trace.py trace.py-20070828055009-7kt0bbc4t4b92apz-1
modified:
bzrlib/registry.py lazy_factory.py-20060809213415-2gfvqadtvdn0phtg-1
bzrlib/tests/test_knit.py test_knit.py-20051212171302-95d4c00dd5f11f2b
bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
bzrlib/transport/__init__.py transport.py-20050711165921-4978aa7ce1285ad5
bzrlib/transport/decorator.py decorator.py-20060402223305-e913a0f25319ab42
=== added file 'bzrlib/transport/trace.py'
--- a/bzrlib/transport/trace.py 1970-01-01 00:00:00 +0000
+++ b/bzrlib/transport/trace.py 2007-10-02 05:33:39 +0000
@@ -0,0 +1,158 @@
+# Copyright (C) 2007 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+"""Implementation of Transport that traces transport operations.
+
+This does not change the transport behaviour at allmerely records every call
+and then delegates it.
+"""
+
+from bzrlib.transport.decorator import TransportDecorator, DecoratorServer
+
+
+class TransportTraceDecorator(TransportDecorator):
+ """A tracing decorator for Transports.
+
+ Calls that potentially perform IO are logged to self._activity. The
+ _activity attribute is shared as the transport is cloned, but not if a new
+ transport is created without cloning.
+ """
+
+ def __init__(self, url, _decorated=None, _from_transport=None):
+ """Set the 'base' path where files will be stored.
+
+ _decorated is a private parameter for cloning.
+ """
+ TransportDecorator.__init__(self, url, _decorated)
+ if _from_transport is None:
+ # newly created
+ self._activity = []
+ else:
+ # cloned
+ self._activity = _from_transport._activity
+
+ def append_file(self, relpath, f, mode=None):
+ """See Transport.append_file()."""
+ return self._decorated.append_file(relpath, f, mode=mode)
+
+ def append_bytes(self, relpath, bytes, mode=None):
+ """See Transport.append_bytes()."""
+ return self._decorated.append_bytes(relpath, bytes, mode=mode)
+
+ def delete(self, relpath):
+ """See Transport.delete()."""
+ return self._decorated.delete(relpath)
+
+ def delete_tree(self, relpath):
+ """See Transport.delete_tree()."""
+ return self._decorated.delete_tree(relpath)
+
+ @classmethod
+ def _get_url_prefix(self):
+ """Tracing transports are identified by 'trace+'"""
+ return 'trace+'
+
+ def get(self, relpath):
+ """See Transport.get()."""
+ self._activity.append(('get', relpath))
+ return self._decorated.get(relpath)
+
+ def get_smart_client(self):
+ return self._decorated.get_smart_client()
+
+ def has(self, relpath):
+ """See Transport.has()."""
+ return self._decorated.has(relpath)
+
+ def is_readonly(self):
+ """See Transport.is_readonly."""
+ return self._decorated.is_readonly()
+
+ def mkdir(self, relpath, mode=None):
+ """See Transport.mkdir()."""
+ return self._decorated.mkdir(relpath, mode)
+
+ def open_write_stream(self, relpath, mode=None):
+ """See Transport.open_write_stream."""
+ return self._decorated.open_write_stream(relpath, mode=mode)
+
+ def put_file(self, relpath, f, mode=None):
+ """See Transport.put_file()."""
+ return self._decorated.put_file(relpath, f, mode)
+
+ def put_bytes(self, relpath, bytes, mode=None):
+ """See Transport.put_bytes()."""
+ self._activity.append(('put_bytes', relpath, len(bytes), mode))
+ return self._decorated.put_bytes(relpath, bytes, mode)
+
+ def listable(self):
+ """See Transport.listable."""
+ return self._decorated.listable()
+
+ def iter_files_recursive(self):
+ """See Transport.iter_files_recursive()."""
+ return self._decorated.iter_files_recursive()
+
+ def list_dir(self, relpath):
+ """See Transport.list_dir()."""
+ return self._decorated.list_dir(relpath)
+
+ def readv(self, relpath, offsets, adjust_for_latency=False,
+ upper_limit=None):
+ """See Transport.readv."""
+ self._activity.append(('readv', relpath, offsets, adjust_for_latency,
+ upper_limit))
+ return self._decorated.readv(relpath, offsets, adjust_for_latency,
+ upper_limit)
+
+ def recommended_page_size(self):
+ """See Transport.recommended_page_size()."""
+ return self._decorated.recommended_page_size()
+
+ def rename(self, rel_from, rel_to):
+ return self._decorated.rename(rel_from, rel_to)
+
+ def rmdir(self, relpath):
+ """See Transport.rmdir."""
+ return self._decorated.rmdir(relpath)
+
+ def stat(self, relpath):
+ """See Transport.stat()."""
+ return self._decorated.stat(relpath)
+
+ def lock_read(self, relpath):
+ """See Transport.lock_read."""
+ return self._decorated.lock_read(relpath)
+
+ def lock_write(self, relpath):
+ """See Transport.lock_write."""
+ return self._decorated.lock_write(relpath)
+
+
+class TraceServer(DecoratorServer):
+ """Server for the TransportTraceDecorator for testing with."""
+
+ def get_decorator_class(self):
+ return TransportTraceDecorator
+
+
+def get_test_permutations():
+ """Return the permutations to be used in testing.
+
+ The Decorator class is not directly usable, and testing it would not have
+ any benefit - its the concrete classes which need to be tested.
+ """
+ return [(TransportTraceDecorator, TraceServer)]
=== modified file 'bzrlib/registry.py'
--- a/bzrlib/registry.py 2007-01-25 00:35:22 +0000
+++ b/bzrlib/registry.py 2007-10-02 05:33:39 +0000
@@ -69,6 +69,11 @@
self._obj = obj
self._imported = True
+ def __repr__(self):
+ return "<%s.%s object at %x, module=%r attribute=%r>" % (
+ self.__class__.__module__, self.__class__.__name__, id(self),
+ self._module_name, self._member_name)
+
class Registry(object):
"""A class that registers objects to a name.
=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py 2007-08-02 23:43:57 +0000
+++ b/bzrlib/tests/test_knit.py 2007-10-02 05:33:39 +0000
@@ -55,7 +55,7 @@
TestCaseWithMemoryTransport,
TestCaseWithTransport,
)
-from bzrlib.transport import TransportLogger, get_transport
+from bzrlib.transport import get_transport
from bzrlib.transport.memory import MemoryTransport
from bzrlib.weave import Weave
@@ -1306,18 +1306,19 @@
k1.get_texts(('%d' % t) for t in range(3))
def test_iter_lines_reads_in_order(self):
- t = MemoryTransport()
- instrumented_t = TransportLogger(t)
+ instrumented_t = get_transport('trace+memory:///')
k1 = KnitVersionedFile('id', instrumented_t, create=True, delta=True)
- self.assertEqual([('id.kndx',)], instrumented_t._calls)
+ self.assertEqual([('get', 'id.kndx',)], instrumented_t._activity)
# add texts with no required ordering
k1.add_lines('base', [], ['text\n'])
k1.add_lines('base2', [], ['text2\n'])
k1.clear_cache()
- instrumented_t._calls = []
+ del instrumented_t._activity[:]
# request a last-first iteration
- results = list(k1.iter_lines_added_or_present_in_versions(['base2', 'base']))
- self.assertEqual([('id.knit', [(0, 87), (87, 89)])], instrumented_t._calls)
+ results = list(k1.iter_lines_added_or_present_in_versions(
+ ['base2', 'base']))
+ self.assertEqual([('readv', 'id.knit', [(0, 87), (87, 89)], False)],
+ instrumented_t._activity)
self.assertEqual(['text\n', 'text2\n'], results)
def test_create_empty_annotated(self):
=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py 2007-08-19 20:38:10 +0000
+++ b/bzrlib/tests/test_transport.py 2007-10-02 05:33:39 +0000
@@ -38,7 +38,8 @@
UnsupportedProtocol,
)
from bzrlib.tests import TestCase, TestCaseInTempDir
-from bzrlib.transport import (_CoalescedOffset,
+from bzrlib.transport import (_clear_protocol_handlers,
+ _CoalescedOffset,
ConnectedTransport,
_get_protocol_handlers,
_set_protocol_handlers,
@@ -73,6 +74,8 @@
def test_get_transport_modules(self):
handlers = _get_protocol_handlers()
+ # don't pollute the current handlers
+ _clear_protocol_handlers()
class SampleHandler(object):
"""I exist, isnt that enough?"""
try:
@@ -89,6 +92,8 @@
def test_transport_dependency(self):
"""Transport with missing dependency causes no error"""
saved_handlers = _get_protocol_handlers()
+ # don't pollute the current handlers
+ _clear_protocol_handlers()
try:
register_transport_proto('foo')
register_lazy_transport('foo', 'bzrlib.tests.test_transport',
@@ -712,11 +717,44 @@
self.assertIsNot(t1, t2)
-def get_test_permutations():
- """Return transport permutations to be used in testing.
-
- This module registers some transports, but they're only for testing
- registration. We don't really want to run all the transport tests against
- them.
- """
- return []
+class TestTransportTrace(TestCase):
+
+ def test_get(self):
+ transport = get_transport('trace+memory://')
+ self.assertIsInstance(
+ transport, bzrlib.transport.trace.TransportTraceDecorator)
+
+ def test_clone_preserves_activity(self):
+ transport = get_transport('trace+memory://')
+ transport2 = transport.clone('.')
+ self.assertTrue(transport is not transport2)
+ self.assertTrue(transport._activity is transport2._activity)
+
+ # the following specific tests are for the operations that have made use of
+ # logging in tests; we could test every single operation but doing that
+ # still won't cause a test failure when the top level Transport API
+ # changes; so there is little return doing that.
+ def test_get(self):
+ transport = get_transport('trace+memory:///')
+ transport.put_bytes('foo', 'barish')
+ transport.get('foo')
+ expected_result = []
+ # put_bytes records the bytes, not the content to avoid memory
+ # pressure.
+ expected_result.append(('put_bytes', 'foo', 6, None))
+ # get records the file name only.
+ expected_result.append(('get', 'foo'))
+ self.assertEqual(expected_result, transport._activity)
+
+ def test_readv(self):
+ transport = get_transport('trace+memory:///')
+ transport.put_bytes('foo', 'barish')
+ list(transport.readv('foo', [(0, 1), (3, 2)], adjust_for_latency=True,
+ upper_limit=6))
+ expected_result = []
+ # put_bytes records the bytes, not the content to avoid memory
+ # pressure.
+ expected_result.append(('put_bytes', 'foo', 6, None))
+ # readv records the supplied offset request
+ expected_result.append(('readv', 'foo', [(0, 1), (3, 2)], True, 6))
+ self.assertEqual(expected_result, transport._activity)
=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py 2007-08-27 04:19:49 +0000
+++ b/bzrlib/tests/test_transport_implementations.py 2007-10-02 05:33:39 +0000
@@ -1498,6 +1498,7 @@
# multiple large random byte look ups we do several tests on the same
# backing data.
content = osutils.rand_bytes(200*1024)
+ content_size = len(content)
if transport.is_readonly():
file('a', 'w').write(content)
else:
@@ -1509,7 +1510,7 @@
# start corner case
result = list(transport.readv('a', ((0, 30),),
- adjust_for_latency=True))
+ adjust_for_latency=True, upper_limit=content_size))
# we expect 1 result, from 0, to something > 30
self.assertEqual(1, len(result))
self.assertEqual(0, result[0][0])
@@ -1517,7 +1518,7 @@
check_result_data(result)
# end of file corner case
result = list(transport.readv('a', ((204700, 100),),
- adjust_for_latency=True))
+ adjust_for_latency=True, upper_limit=content_size))
# we expect 1 result, from 204800- its length, to the end
self.assertEqual(1, len(result))
data_len = len(result[0][1])
@@ -1526,7 +1527,7 @@
check_result_data(result)
# out of order ranges are made in order
result = list(transport.readv('a', ((204700, 100), (0, 50)),
- adjust_for_latency=True))
+ adjust_for_latency=True, upper_limit=content_size))
# we expect 2 results, in order, start and end.
self.assertEqual(2, len(result))
# start
@@ -1541,7 +1542,7 @@
# close ranges get combined (even if out of order)
for request_vector in [((400,50), (800, 234)), ((800, 234), (400,50))]:
result = list(transport.readv('a', request_vector,
- adjust_for_latency=True))
+ adjust_for_latency=True, upper_limit=content_size))
self.assertEqual(1, len(result))
data_len = len(result[0][1])
# minimmum length is from 400 to 1034 - 634
=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py 2007-08-27 04:19:49 +0000
+++ b/bzrlib/transport/__init__.py 2007-10-02 05:33:39 +0000
@@ -99,7 +99,7 @@
modules.add(factory._module_name)
else:
modules.add(factory._obj.__module__)
- # Add chroot directly, because there is not handler registered for it.
+ # Add chroot directly, because there is no handler registered for it.
modules.add('bzrlib.transport.chroot')
result = list(modules)
result.sort()
@@ -125,7 +125,7 @@
self.get(key).insert(0, registry._ObjectGetter(obj))
def register_lazy_transport_provider(self, key, module_name, member_name):
- self.get(key).insert(0,
+ self.get(key).insert(0,
registry._LazyObjectGetter(module_name, member_name))
def register_transport(self, key, help=None, info=None):
@@ -136,7 +136,7 @@
self._default_key = key
-transport_list_registry = TransportListRegistry( )
+transport_list_registry = TransportListRegistry()
def register_transport_proto(prefix, help=None, info=None):
@@ -635,7 +635,8 @@
"""
raise errors.NoSmartMedium(self)
- def readv(self, relpath, offsets, adjust_for_latency=False):
+ def readv(self, relpath, offsets, adjust_for_latency=False,
+ upper_limit=None):
"""Get parts of the file at the given relative path.
:param relpath: The path to read data from.
@@ -644,6 +645,13 @@
transport latency. This may re-order the offsets, expand them to
grab adjacent data when there is likely a high cost to requesting
data relative to delivering it.
+ :param upper_limit: When adjust_for_latency is True setting upper_limit
+ allows the caller to tell the transport about the length of the
+ file, so that requests are not issued for ranges beyond the end of
+ the file. This matters because some servers and/or transports error
+ in such a case rather than just satisfying the available ranges.
+ upper_limit should always be provided when adjust_for_latency is
+ True, and should be the size of the file in bytes.
:return: A list or generator of (offset, data) tuples
"""
if adjust_for_latency:
@@ -658,15 +666,19 @@
yield None
return empty_yielder()
# expand by page size at either end
- expansion = self.recommended_page_size() / 2
+ expansion = self.recommended_page_size()
+ reduction = expansion / 2
new_offsets = []
for offset, length in offsets:
- new_offset = offset - expansion
+ new_offset = offset - reduction
new_length = length + expansion
if new_offset < 0:
# don't ask for anything < 0
new_length -= new_offset
new_offset = 0
+ if (upper_limit is not None and
+ new_offset + new_length > upper_limit):
+ new_length = upper_limit - new_offset
new_offsets.append((new_offset, new_length))
# combine the expanded offsets
offsets = []
@@ -687,7 +699,8 @@
def _readv(self, relpath, offsets):
"""Get parts of the file at the given relative path.
- :offsets: A list of (offset, size) tuples.
+ :param relpath: The path to read.
+ :param offsets: A list of (offset, size) tuples.
:return: A list or generator of (offset, data) tuples
"""
if not offsets:
@@ -1644,30 +1657,6 @@
raise NotImplementedError
-class TransportLogger(object):
- """Adapt a transport to get clear logging data on api calls.
-
- Feel free to extend to log whatever calls are of interest.
- """
-
- def __init__(self, adapted):
- self._adapted = adapted
- self._calls = []
-
- def get(self, name):
- self._calls.append((name,))
- return self._adapted.get(name)
-
- def __getattr__(self, name):
- """Thunk all undefined access through to self._adapted."""
- # raise AttributeError, name
- return getattr(self._adapted, name)
-
- def readv(self, name, offsets):
- self._calls.append((name, offsets))
- return self._adapted.readv(name, offsets)
-
-
# None is the default transport, for things with no url scheme
register_transport_proto('file://',
help="Access using the standard filesystem (default)")
@@ -1719,6 +1708,8 @@
register_transport_proto('memory://')
register_lazy_transport('memory://', 'bzrlib.transport.memory', 'MemoryTransport')
+
+# chroots cannot be implicitly accessed, they must be explicitly created:
register_transport_proto('chroot+')
register_transport_proto('readonly+',
@@ -1729,6 +1720,9 @@
register_transport_proto('fakenfs+')
register_lazy_transport('fakenfs+', 'bzrlib.transport.fakenfs', 'FakeNFSTransportDecorator')
+register_transport_proto('trace+')
+register_lazy_transport('trace+', 'bzrlib.transport.trace', 'TransportTraceDecorator')
+
register_transport_proto('unlistable+')
register_lazy_transport('unlistable+', 'bzrlib.transport.unlistable', 'UnlistableTransportDecorator')
=== modified file 'bzrlib/transport/decorator.py'
--- a/bzrlib/transport/decorator.py 2007-08-26 22:10:51 +0000
+++ b/bzrlib/transport/decorator.py 2007-10-02 05:33:39 +0000
@@ -37,10 +37,13 @@
method to return the url prefix for the subclass.
"""
- def __init__(self, url, _decorated=None):
- """Set the 'base' path where files will be stored.
+ def __init__(self, url, _decorated=None, _from_transport=None):
+ """Set the 'base' path of the transport.
- _decorated is a private parameter for cloning."""
+ :param _decorated: A private parameter for cloning.
+ :param _from_transport: Is available for subclasses that
+ need to share state across clones.
+ """
prefix = self._get_url_prefix()
assert url.startswith(prefix), \
"url %r doesn't start with decorator prefix %r" % \
@@ -73,7 +76,8 @@
"""See Transport.clone()."""
decorated_clone = self._decorated.clone(offset)
return self.__class__(
- self._get_url_prefix() + decorated_clone.base, decorated_clone)
+ self._get_url_prefix() + decorated_clone.base, decorated_clone,
+ self)
def delete(self, relpath):
"""See Transport.delete()."""
More information about the bazaar-commits
mailing list