Merge lp:~spiv/bzr/hpss-perm-denied-during-push-create into lp:~bzr/bzr/trunk-old

Proposed by Andrew Bennetts on 2009-06-02
Status: Merged
Approved by: John A Meinel on 2009-06-02
Approved revision: 4386
Merged at revision: not available
Proposed branch: lp:~spiv/bzr/hpss-perm-denied-during-push-create
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 119 lines
To merge this branch: bzr merge lp:~spiv/bzr/hpss-perm-denied-during-push-create
Reviewer Review Type Date Requested Status
John A Meinel 2009-06-02 Approve on 2009-06-02
Review via email: mp+6959@code.launchpad.net
To post a comment you must log in.
Andrew Bennetts (spiv) wrote :

Catches and translates ErrorFromSmartServer in RemoteBzrDirFormat. This makes it consistent with other callers of _SmartClient.call (in bzrlib.remote and bzrlib.transport.remote), and fixes ugly tracebacks when creating new BzrDirs on a smart server.

This also adds some very basic unit tests for the BzrDirFormat.initialize_ex verb, and refactors the RemoteBzrDirFormat.initialize_on_transport_ex a little to make that testing easier. The test coverage here is far from great, but it is a least a small improvement.

John A Meinel (jameinel) wrote :

I thought we also were having some problems with "NoSuchDirectory" which seems a lot easier to test. Just use "trans = self.make_smart_server('path')" and then call initialize_on_transport_ex with a subdir.

Anyway, the change itself seems ok.

review: Approve
4387. By Andrew Bennetts on 2009-06-03

Add test suggested by John.

4388. By Andrew Bennetts on 2009-06-03

Merge, resolving NEWS conflict.

4389. By Andrew Bennetts on 2009-06-03

Merge, resolving NEWS conflict.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-06-03 05:44:18 +0000
+++ NEWS 2009-06-03 07:35:51 +0000
@@ -62,6 +62,10 @@
62 succeeded from the user's perspective, no output is written to stderr 62 succeeded from the user's perspective, no output is written to stderr
63 or stdout. (Maritza Mendez, #363837)63 or stdout. (Maritza Mendez, #363837)
6464
65* Translate errors received from a smart server in response to a
66 ``BzrDirFormat.initialize`` or ``BzrDirFormat.initialize_ex`` request.
67 This was causing tracebacks even for mundane errors like
68 ``PermissionDenied``. (Andrew Bennetts, #381329)
6569
66Documentation70Documentation
67*************71*************
6872
=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py 2009-05-28 11:46:14 +0000
+++ bzrlib/bzrdir.py 2009-06-03 07:35:51 +0000
@@ -3049,7 +3049,10 @@
3049 return local_dir_format.initialize_on_transport(transport)3049 return local_dir_format.initialize_on_transport(transport)
3050 client = _SmartClient(client_medium)3050 client = _SmartClient(client_medium)
3051 path = client.remote_path_from_transport(transport)3051 path = client.remote_path_from_transport(transport)
3052 response = client.call('BzrDirFormat.initialize', path)3052 try:
3053 response = client.call('BzrDirFormat.initialize', path)
3054 except errors.ErrorFromSmartServer, err:
3055 remote._translate_error(err, path=path)
3053 if response[0] != 'ok':3056 if response[0] != 'ok':
3054 raise errors.SmartProtocolError('unexpected response code %s' % (response,))3057 raise errors.SmartProtocolError('unexpected response code %s' % (response,))
3055 format = RemoteBzrDirFormat()3058 format = RemoteBzrDirFormat()
@@ -3115,6 +3118,13 @@
3115 stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,3118 stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
3116 make_working_trees=make_working_trees, shared_repo=shared_repo,3119 make_working_trees=make_working_trees, shared_repo=shared_repo,
3117 vfs_only=True)3120 vfs_only=True)
3121 return self._initialize_on_transport_ex_rpc(client, path, transport,
3122 use_existing_dir, create_prefix, force_new_repo, stacked_on,
3123 stack_on_pwd, repo_format_name, make_working_trees, shared_repo)
3124
3125 def _initialize_on_transport_ex_rpc(self, client, path, transport,
3126 use_existing_dir, create_prefix, force_new_repo, stacked_on,
3127 stack_on_pwd, repo_format_name, make_working_trees, shared_repo):
3118 args = []3128 args = []
3119 args.append(self._serialize_NoneTrueFalse(use_existing_dir))3129 args.append(self._serialize_NoneTrueFalse(use_existing_dir))
3120 args.append(self._serialize_NoneTrueFalse(create_prefix))3130 args.append(self._serialize_NoneTrueFalse(create_prefix))
@@ -3147,6 +3157,8 @@
3147 stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,3157 stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
3148 make_working_trees=make_working_trees, shared_repo=shared_repo,3158 make_working_trees=make_working_trees, shared_repo=shared_repo,
3149 vfs_only=True)3159 vfs_only=True)
3160 except errors.ErrorFromSmartServer, err:
3161 remote._translate_error(err, path=path)
3150 repo_path = response[0]3162 repo_path = response[0]
3151 bzrdir_name = response[6]3163 bzrdir_name = response[6]
3152 require_stacking = response[7]3164 require_stacking = response[7]
31533165
=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py 2009-05-07 05:08:46 +0000
+++ bzrlib/tests/test_remote.py 2009-06-03 07:35:51 +0000
@@ -741,6 +741,61 @@
741 self.assertEqual(network_name, repo._format.network_name())741 self.assertEqual(network_name, repo._format.network_name())
742742
743743
744class TestBzrDirFormatInitializeEx(TestRemote):
745
746 def test_success(self):
747 """Simple test for typical successful call."""
748 fmt = bzrdir.RemoteBzrDirFormat()
749 default_format_name = BzrDirFormat.get_default_format().network_name()
750 transport = self.get_transport()
751 client = FakeClient(transport.base)
752 client.add_expected_call(
753 'BzrDirFormat.initialize_ex',
754 (default_format_name, 'path', 'False', 'False', 'False', '',
755 '', '', '', 'False'),
756 'success',
757 ('.', 'no', 'no', 'yes', 'repo fmt', 'repo bzrdir fmt',
758 'bzrdir fmt', 'False', '', '', 'repo lock token'))
759 # XXX: It would be better to call fmt.initialize_on_transport_ex, but
760 # it's currently hard to test that without supplying a real remote
761 # transport connected to a real server.
762 result = fmt._initialize_on_transport_ex_rpc(client, 'path',
763 transport, False, False, False, None, None, None, None, False)
764 client.finished_test()
765
766 def test_error(self):
767 """Error responses are translated, e.g. 'PermissionDenied' raises the
768 corresponding error from the client.
769 """
770 fmt = bzrdir.RemoteBzrDirFormat()
771 default_format_name = BzrDirFormat.get_default_format().network_name()
772 transport = self.get_transport()
773 client = FakeClient(transport.base)
774 client.add_expected_call(
775 'BzrDirFormat.initialize_ex',
776 (default_format_name, 'path', 'False', 'False', 'False', '',
777 '', '', '', 'False'),
778 'error',
779 ('PermissionDenied', 'path', 'extra info'))
780 # XXX: It would be better to call fmt.initialize_on_transport_ex, but
781 # it's currently hard to test that without supplying a real remote
782 # transport connected to a real server.
783 err = self.assertRaises(errors.PermissionDenied,
784 fmt._initialize_on_transport_ex_rpc, client, 'path', transport,
785 False, False, False, None, None, None, None, False)
786 self.assertEqual('path', err.path)
787 self.assertEqual(': extra info', err.extra)
788 client.finished_test()
789
790 def test_error_from_real_server(self):
791 """Integration test for error translation."""
792 transport = self.make_smart_server('foo')
793 transport = transport.clone('no-such-path')
794 fmt = bzrdir.RemoteBzrDirFormat()
795 err = self.assertRaises(errors.NoSuchFile,
796 fmt.initialize_on_transport_ex, transport, create_prefix=False)
797
798
744class OldSmartClient(object):799class OldSmartClient(object):
745 """A fake smart client for test_old_version that just returns a version one800 """A fake smart client for test_old_version that just returns a version one
746 response to the 'hello' (query version) command.801 response to the 'hello' (query version) command.