Merge lp:~gz/bzr/2.3_unprintable_retrywithnewpacks into lp:bzr/2.3

Proposed by Martin Packman on 2011-11-10
Status: Merged
Approved by: Vincent Ladeuil on 2011-11-10
Approved revision: 5664
Merged at revision: 5664
Proposed branch: lp:~gz/bzr/2.3_unprintable_retrywithnewpacks
Merge into: lp:bzr/2.3
Diff against target: 43 lines (+12/-0)
3 files modified
bzrlib/errors.py (+1/-0)
bzrlib/tests/test_errors.py (+8/-0)
doc/en/release-notes/bzr-2.3.txt (+3/-0)
To merge this branch: bzr merge lp:~gz/bzr/2.3_unprintable_retrywithnewpacks
Reviewer Review Type Date Requested Status
Vincent Ladeuil 2011-11-10 Approve on 2011-11-10
Review via email: mp+81827@code.launchpad.net

Commit message

Store context argument to RetryWithNewPacks which stops stringification breaking

Description of the change

Merge Andrew's fix for the RetryWithNewPacks exception not being printable, due to missing the context attribute. The exception is not intended to bubble up to users at all, and this doesn't tackle the underlying problem in bug 709349 but may as well be resolved separately now.

Proposing against 2.3 as that's what his branch was, and it's a safe and obvious fix.

To post a comment you must log in.
Vincent Ladeuil (vila) wrote :

Safe and obvious indeed, green light to merge in 2.3 and up to trunk

Just one question:

26 + '{context} {exc value}', str(error))

is error not an exception or is it safe to call str() here ? If the later, I'll warmly welcome some idiot-proof guide on when that's the case ;)

review: Approve
Martin Packman (gz) wrote :

> Just one question:
>
> 26 + '{context} {exc value}', str(error))
>
> is error not an exception or is it safe to call str() here ? If the later,
> I'll warmly welcome some idiot-proof guide on when that's the case ;)

Test does look a bit funny, but works with how the code is written currently. Calling str() is okay because we're supplying ascii only inputs - and it's one of our exceptions that has the added robustness when formatting, hence the original 'Unprintable exception' complaint rather than a knock-on KeyError.

Test is in the right place to get updated on bzr.dev when handling of wrapped exceptions is improved.

Martin Packman (gz) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/errors.py'
2--- bzrlib/errors.py 2011-07-27 11:57:32 +0000
3+++ bzrlib/errors.py 2011-11-10 11:06:39 +0000
4@@ -1563,6 +1563,7 @@
5 problem we can raise the original error (value from sys.exc_info())
6 """
7 BzrError.__init__(self)
8+ self.context = context
9 self.reload_occurred = reload_occurred
10 self.exc_info = exc_info
11 self.orig_error = exc_info[1]
12
13=== modified file 'bzrlib/tests/test_errors.py'
14--- bzrlib/tests/test_errors.py 2011-01-12 01:01:53 +0000
15+++ bzrlib/tests/test_errors.py 2011-11-10 11:06:39 +0000
16@@ -712,6 +712,14 @@
17 'Please use `bzr unbind` to fix.')
18 self.assertEqualDiff(msg, str(error))
19
20+ def test_retry_with_new_packs(self):
21+ fake_exc_info = ('{exc type}', '{exc value}', '{exc traceback}')
22+ error = errors.RetryWithNewPacks(
23+ '{context}', reload_occurred=False, exc_info=fake_exc_info)
24+ self.assertEqual(
25+ 'Pack files have changed, reload and retry. context: '
26+ '{context} {exc value}', str(error))
27+
28
29 class PassThroughError(errors.BzrError):
30
31
32=== modified file 'doc/en/release-notes/bzr-2.3.txt'
33--- doc/en/release-notes/bzr-2.3.txt 2011-09-09 12:32:08 +0000
34+++ doc/en/release-notes/bzr-2.3.txt 2011-11-10 11:06:39 +0000
35@@ -35,6 +35,9 @@
36 * Cope cleanly with buggy HTTP proxies that close the socket in the middle
37 of a multipart response. (Martin Pool, #198646).
38
39+* Fix "Unprintable exception" error when a RetryWithNewPacks error is
40+ displayed. (Andrew Bennetts)
41+
42 Documentation
43 *************
44

Subscribers

People subscribed via source and target branches