Launchpad itself

Merge lp:~thumper/launchpad/lp-distroseries into lp:launchpad

  1. lp-distroseries
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 9870
Proposed branch: lp:~thumper/launchpad/lp-distroseries
Merge into: lp:launchpad
Diff against target: 68 lines (+17/-1)
4 files modified
lib/canonical/launchpad/xmlrpc/faults.py (+1/-1)
lib/lp/code/interfaces/linkedbranch.py (+5/-0)
lib/lp/code/xmlrpc/tests/test_branch.py (+8/-0)
lib/lp/testing/factory.py (+3/-0)
To merge this branch: bzr merge lp:~thumper/launchpad/lp-distroseries
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+14777@code.launchpad.net

Commit message

Don't oops if resolving a distroseries for lp: traversal.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

A relatively simple branch that addresses bug 480024.

tests:
  TestExpandURL

QA:
  bzr revno lp:ubuntu/karmic
or
  bzr revno lp://dev/ubuntu/warty

Gets a nice error like:
$ bzr revno lp://dev/ubuntu/warty
bzr: ERROR: Invalid url supplied to transport: "lp://dev/ubuntu/warty": Warty is a distroseries, and a distroseries doesn't have a default branch.

instead of an oops.

Revision history for this message
Jonathan Lange (jml) wrote :

Thanks for fixing this Tim. The fix is good, and I like the new name for makeDistroRelease.

However, I think the error message will be bogus if you try lp://dev/ubuntu/warty-backports. Could you please add a test for this?

review: Needs Fixing
Revision history for this message
Tim Penhey (thumper) wrote :

On Thu, 12 Nov 2009 18:32:17 Jonathan Lange wrote:
> Review: Needs Fixing
> Thanks for fixing this Tim. The fix is good, and I like the new name for
> makeDistroRelease.
>
> However, I think the error message will be bogus if you try
> lp://dev/ubuntu/warty-backports. Could you please add a test for this?

It says:

$ bzr revno lp://dev/ubuntu/warty-backports
bzr: ERROR: Invalid url supplied to transport: "lp://dev/ubuntu/warty-
backports": Warty is a distroseries, and a distroseries doesn't have a default
branch.

It is still valid, but doesn't show the relationship between "Warty" and
"warty-backports". Suggestions?

Revision history for this message
Jonathan Lange (jml) wrote :

I think it's fine. Could you please change the fault text to say "cannot" rather than "doesn't"?

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

On Thu, 12 Nov 2009 19:00:19 Jonathan Lange wrote:
> Review: Approve
> I think it's fine. Could you please change the fault text to say "cannot"
> rather than "doesn't"?

OK.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/xmlrpc/faults.py'
2--- lib/canonical/launchpad/xmlrpc/faults.py 2009-06-25 05:30:52 +0000
3+++ lib/canonical/launchpad/xmlrpc/faults.py 2009-11-12 06:30:31 +0000
4@@ -309,7 +309,7 @@
5 error_code = 230
6 msg_template = (
7 "%(component_name)s is a %(component_type)s, and a "
8- "%(component_type)s doesn't have a default branch.")
9+ "%(component_type)s cannot have a default branch.")
10
11 def __init__(self, component):
12 LaunchpadFault.__init__(
13
14=== modified file 'lib/lp/code/interfaces/linkedbranch.py'
15--- lib/lp/code/interfaces/linkedbranch.py 2009-07-19 23:17:34 +0000
16+++ lib/lp/code/interfaces/linkedbranch.py 2009-11-12 06:30:31 +0000
17@@ -19,6 +19,7 @@
18 ]
19
20 from zope.interface import Attribute, Interface
21+from zope.security.proxy import isinstance as zope_isinstance
22
23
24 class ICanHasLinkedBranch(Interface):
25@@ -67,6 +68,10 @@
26 """
27 has_linked_branch = ICanHasLinkedBranch(provided, None)
28 if has_linked_branch is None:
29+ if zope_isinstance(provided, tuple):
30+ # Distroseries are returned as tuples containing distroseries and
31+ # pocket.
32+ provided = provided[0]
33 raise CannotHaveLinkedBranch(provided)
34 branch = has_linked_branch.branch
35 if branch is None:
36
37=== modified file 'lib/lp/code/xmlrpc/tests/test_branch.py'
38--- lib/lp/code/xmlrpc/tests/test_branch.py 2009-07-17 00:26:05 +0000
39+++ lib/lp/code/xmlrpc/tests/test_branch.py 2009-11-12 06:30:31 +0000
40@@ -130,6 +130,14 @@
41 distro = self.factory.makeDistribution()
42 self.assertFault(distro.name, faults.CannotHaveLinkedBranch(distro))
43
44+ def test_distroseries_name(self):
45+ # Resolving lp:///distro/series' should explain that distribution
46+ # series don't have default branches.
47+ series = self.factory.makeDistroSeries()
48+ self.assertFault(
49+ '%s/%s' % (series.distribution.name, series.name),
50+ faults.CannotHaveLinkedBranch(series))
51+
52 def test_invalid_product_name(self):
53 # If we get a string that cannot be a name for a product where we
54 # expect the name of a product, we should error appropriately.
55
56=== modified file 'lib/lp/testing/factory.py'
57--- lib/lp/testing/factory.py 2009-10-30 17:32:39 +0000
58+++ lib/lp/testing/factory.py 2009-11-12 06:30:31 +0000
59@@ -1435,6 +1435,9 @@
60 series.status = status
61 return series
62
63+ # Most people think of distro releases as distro series.
64+ makeDistroSeries = makeDistroRelease
65+
66 def makeDistroArchSeries(self, distroseries=None,
67 architecturetag='powerpc', processorfamily=None,
68 official=True, owner=None,