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
=== modified file 'lib/canonical/launchpad/xmlrpc/faults.py'
--- lib/canonical/launchpad/xmlrpc/faults.py 2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/xmlrpc/faults.py 2009-11-12 06:30:31 +0000
@@ -309,7 +309,7 @@
309 error_code = 230309 error_code = 230
310 msg_template = (310 msg_template = (
311 "%(component_name)s is a %(component_type)s, and a "311 "%(component_name)s is a %(component_type)s, and a "
312 "%(component_type)s doesn't have a default branch.")312 "%(component_type)s cannot have a default branch.")
313313
314 def __init__(self, component):314 def __init__(self, component):
315 LaunchpadFault.__init__(315 LaunchpadFault.__init__(
316316
=== modified file 'lib/lp/code/interfaces/linkedbranch.py'
--- lib/lp/code/interfaces/linkedbranch.py 2009-07-19 23:17:34 +0000
+++ lib/lp/code/interfaces/linkedbranch.py 2009-11-12 06:30:31 +0000
@@ -19,6 +19,7 @@
19 ]19 ]
2020
21from zope.interface import Attribute, Interface21from zope.interface import Attribute, Interface
22from zope.security.proxy import isinstance as zope_isinstance
2223
2324
24class ICanHasLinkedBranch(Interface):25class ICanHasLinkedBranch(Interface):
@@ -67,6 +68,10 @@
67 """68 """
68 has_linked_branch = ICanHasLinkedBranch(provided, None)69 has_linked_branch = ICanHasLinkedBranch(provided, None)
69 if has_linked_branch is None:70 if has_linked_branch is None:
71 if zope_isinstance(provided, tuple):
72 # Distroseries are returned as tuples containing distroseries and
73 # pocket.
74 provided = provided[0]
70 raise CannotHaveLinkedBranch(provided)75 raise CannotHaveLinkedBranch(provided)
71 branch = has_linked_branch.branch76 branch = has_linked_branch.branch
72 if branch is None:77 if branch is None:
7378
=== modified file 'lib/lp/code/xmlrpc/tests/test_branch.py'
--- lib/lp/code/xmlrpc/tests/test_branch.py 2009-07-17 00:26:05 +0000
+++ lib/lp/code/xmlrpc/tests/test_branch.py 2009-11-12 06:30:31 +0000
@@ -130,6 +130,14 @@
130 distro = self.factory.makeDistribution()130 distro = self.factory.makeDistribution()
131 self.assertFault(distro.name, faults.CannotHaveLinkedBranch(distro))131 self.assertFault(distro.name, faults.CannotHaveLinkedBranch(distro))
132132
133 def test_distroseries_name(self):
134 # Resolving lp:///distro/series' should explain that distribution
135 # series don't have default branches.
136 series = self.factory.makeDistroSeries()
137 self.assertFault(
138 '%s/%s' % (series.distribution.name, series.name),
139 faults.CannotHaveLinkedBranch(series))
140
133 def test_invalid_product_name(self):141 def test_invalid_product_name(self):
134 # If we get a string that cannot be a name for a product where we142 # If we get a string that cannot be a name for a product where we
135 # expect the name of a product, we should error appropriately.143 # expect the name of a product, we should error appropriately.
136144
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2009-10-30 17:32:39 +0000
+++ lib/lp/testing/factory.py 2009-11-12 06:30:31 +0000
@@ -1435,6 +1435,9 @@
1435 series.status = status1435 series.status = status
1436 return series1436 return series
14371437
1438 # Most people think of distro releases as distro series.
1439 makeDistroSeries = makeDistroRelease
1440
1438 def makeDistroArchSeries(self, distroseries=None,1441 def makeDistroArchSeries(self, distroseries=None,
1439 architecturetag='powerpc', processorfamily=None,1442 architecturetag='powerpc', processorfamily=None,
1440 official=True, owner=None,1443 official=True, owner=None,