Bazaar

Merge lp:~spiv/bzr/hpss-compat-528041 into lp:bzr/2.1

  1. hpss-compat-528041
  2. Merge into 2.1
Proposed by Andrew Bennetts
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~spiv/bzr/hpss-compat-528041
Merge into: lp:bzr/2.1
Diff against target: 101 lines (+50/-5)
3 files modified
NEWS (+14/-0)
bzrlib/smart/medium.py (+8/-2)
bzrlib/tests/test_remote.py (+28/-3)
To merge this branch: bzr merge lp:~spiv/bzr/hpss-compat-528041
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+24202@code.launchpad.net

Description of the change

This fixes the AssertionError from _remember_remote_is_before when opening a branch from a 1.5 (or older) server. The right fix is to simply not raise an error there, all it does is inflict pain on users for a minor (at worst) performance bug. The longer analysis is at <https://bugs.edge.launchpad.net/bzr/+bug/528041/comments/11> if you're curious.

I'm sure Martin was hoping for this method to be gone entirely, but hopefully this smaller step will still make him at least a little be happy :)

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

Sounds good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-04-30 06:27:15 +0000
+++ NEWS 2010-04-30 08:16:29 +0000
@@ -16,12 +16,26 @@
16* ``bzr switch`` does not die if a ConfigurableFileMerger is used.16* ``bzr switch`` does not die if a ConfigurableFileMerger is used.
17 (Aaron Bentley, #559436)17 (Aaron Bentley, #559436)
1818
19* Fixed ``AssertionError`` when accessing smart servers running Bazaar
20 versions before 1.6.
21 (Andrew Bennetts, #528041)
22
19* Reset ``siginterrupt`` flag to False every time we handle a signal23* Reset ``siginterrupt`` flag to False every time we handle a signal
20 installed with ``set_signal_handler(..., restart_syscall=True)`` (from24 installed with ``set_signal_handler(..., restart_syscall=True)`` (from
21 ``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"25 ``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"
22 errors after two window resizes.26 errors after two window resizes.
23 (Andrew Bennetts)27 (Andrew Bennetts)
2428
29Internals
30*********
31
32* ``_remember_remote_is_before`` no longer raises AssertionError when
33 suboptimal network behaviour is noticed; instead it just mutters to the
34 log file (and warns the user if they have set the ``hpss`` debug flag).
35 This was causing unnecessary aborts for performance bugs that are minor
36 at worst.
37 (Andrew Bennetts, #528041)
38
2539
26bzr 2.1.140bzr 2.1.1
27#########41#########
2842
=== modified file 'bzrlib/smart/medium.py'
--- bzrlib/smart/medium.py 2010-02-17 17:11:16 +0000
+++ bzrlib/smart/medium.py 2010-04-30 08:16:29 +0000
@@ -609,10 +609,16 @@
609 # which is newer than a previously supplied older-than version.609 # which is newer than a previously supplied older-than version.
610 # This indicates that some smart verb call is not guarded610 # This indicates that some smart verb call is not guarded
611 # appropriately (it should simply not have been tried).611 # appropriately (it should simply not have been tried).
612 raise AssertionError(612 trace.mutter(
613 "_remember_remote_is_before(%r) called, but "613 "_remember_remote_is_before(%r) called, but "
614 "_remember_remote_is_before(%r) was called previously."614 "_remember_remote_is_before(%r) was called previously."
615 % (version_tuple, self._remote_version_is_before))615 , version_tuple, self._remote_version_is_before)
616 if 'hpss' in debug.debug_flags:
617 ui.ui_factory.show_warning(
618 "_remember_remote_is_before(%r) called, but "
619 "_remember_remote_is_before(%r) was called previously."
620 % (version_tuple, self._remote_version_is_before))
621 return
616 self._remote_version_is_before = version_tuple622 self._remote_version_is_before = version_tuple
617623
618 def protocol_version(self):624 def protocol_version(self):
619625
=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py 2010-04-23 05:28:35 +0000
+++ bzrlib/tests/test_remote.py 2010-04-30 08:16:29 +0000
@@ -417,9 +417,12 @@
417 # Calling _remember_remote_is_before again with a lower value works.417 # Calling _remember_remote_is_before again with a lower value works.
418 client_medium._remember_remote_is_before((1, 5))418 client_medium._remember_remote_is_before((1, 5))
419 self.assertTrue(client_medium._is_remote_before((1, 5)))419 self.assertTrue(client_medium._is_remote_before((1, 5)))
420 # You cannot call _remember_remote_is_before with a larger value.420 # If you call _remember_remote_is_before with a higher value it logs a
421 self.assertRaises(421 # warning, and continues to remember the lower value.
422 AssertionError, client_medium._remember_remote_is_before, (1, 9))422 self.assertNotContainsRe(self.get_log(), '_remember_remote_is_before')
423 client_medium._remember_remote_is_before((1, 9))
424 self.assertContainsRe(self.get_log(), '_remember_remote_is_before')
425 self.assertTrue(client_medium._is_remote_before((1, 5)))
423426
424427
425class TestBzrDirCloningMetaDir(TestRemote):428class TestBzrDirCloningMetaDir(TestRemote):
@@ -527,6 +530,28 @@
527 self.assertIsInstance(bd, RemoteBzrDir)530 self.assertIsInstance(bd, RemoteBzrDir)
528 self.assertFinished(client)531 self.assertFinished(client)
529532
533 def test_backwards_compat_hpss_v2(self):
534 client, transport = self.make_fake_client_and_transport()
535 # Monkey-patch fake client to simulate real-world behaviour with v2
536 # server: upon first RPC call detect the protocol version, and because
537 # the version is 2 also do _remember_remote_is_before((1, 6)) before
538 # continuing with the RPC.
539 orig_check_call = client._check_call
540 def check_call(method, args):
541 client._medium._protocol_version = 2
542 client._medium._remember_remote_is_before((1, 6))
543 client._check_call = orig_check_call
544 client._check_call(method, args)
545 client._check_call = check_call
546 client.add_expected_call(
547 'BzrDir.open_2.1', ('quack/',), 'unknown', ('BzrDir.open_2.1',))
548 client.add_expected_call(
549 'BzrDir.open', ('quack/',), 'success', ('yes',))
550 bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
551 _client=client, _force_probe=True)
552 self.assertIsInstance(bd, RemoteBzrDir)
553 self.assertFinished(client)
554
530555
531class TestBzrDirOpenBranch(TestRemote):556class TestBzrDirOpenBranch(TestRemote):
532557

Subscribers

People subscribed via source and target branches