Launchpad itself

Merge lp://staging/~michael.nelson/launchpad/496862-ppa-installable-binaries into lp://staging/launchpad

  1. 496862-ppa-installable-binaries
  2. Merge into devel
Proposed by Michael Nelson
Status: Rejected
Rejected by: Michael Nelson
Proposed branch: lp://staging/~michael.nelson/launchpad/496862-ppa-installable-binaries
Merge into: lp://staging/launchpad
Diff against target: 675 lines (+240/-89)
10 files modified
lib/lp/soyuz/browser/archive.py (+79/-40)
lib/lp/soyuz/browser/tests/archive-views.txt (+51/-37)
lib/lp/soyuz/browser/tests/test_archive_view.py (+59/-0)
lib/lp/soyuz/doc/archive.txt (+14/-4)
lib/lp/soyuz/interfaces/archive.py (+7/-0)
lib/lp/soyuz/model/archive.py (+23/-1)
lib/lp/soyuz/templates/archive-copy-packages.pt (+1/-1)
lib/lp/soyuz/templates/archive-delete-packages.pt (+1/-1)
lib/lp/soyuz/templates/archive-index.pt (+1/-1)
lib/lp/soyuz/templates/archive-macros.pt (+4/-4)
To merge this branch: bzr merge lp://staging/~michael.nelson/launchpad/496862-ppa-installable-binaries
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+21623@code.staging.launchpad.net

Description of the change

This branch is the first of two to address bug 496862.

It adds an ArchiveArchFactory to create a vocabulary dynamically depending on the binaries published in the archive.

The ArchiveSourcePackageListViewBase class is refactored to ArchivePackagesViewBase so that it can be re-used for both source and binary packages.

Finally, the ArchiveView (for the PPA index page) is updated to present binary packages.

The next branch will switch the PPA index ui to present the binary packages.

To test:
bin/test -vv -t archive-views.txt -t archive.txt -t TestArchiveView

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (4.6 KiB)

On Thu, Mar 18, 2010 at 9:24 AM, Michael Nelson
<email address hidden> wrote:
> Michael Nelson has proposed merging lp:~michael.nelson/launchpad/496862-ppa-installable-binaries into lp:launchpad/devel.
>
> Requested reviews:
>  Canonical Launchpad Engineering (launchpad)
>
>
> This branch is the first of two to address bug 496862.
>
> It adds an ArchiveArchFactory to create a vocabulary dynamically depending on the binaries published in the archive.
>
> The ArchiveSourcePackageListViewBase class is refactored to ArchivePackagesViewBase so that it can be re-used for both source and binary packages.
>
> Finally, the ArchiveView (for the PPA index page) is updated to present binary packages.
>
> The next branch will switch the PPA index ui to present the binary packages.
>
> To test:
> bin/test -vv -t archive-views.txt -t archive.txt -t TestArchiveView
>

Good patch. I like the tests and the renames.

Some minor issues with method names and docstring formatting and a
couple of questions.

jml

> --
> https://code.launchpad.net/~michael.nelson/launchpad/496862-ppa-installable-binaries/+merge/21623
> Your team Launchpad code reviewers from Canonical is subscribed to branch lp:launchpad/devel.
>
> === modified file 'lib/lp/soyuz/browser/archive.py'
> --- lib/lp/soyuz/browser/archive.py     2010-03-10 12:50:18 +0000
> +++ lib/lp/soyuz/browser/archive.py     2010-03-18 09:24:24 +0000
> @@ -589,6 +589,26 @@
>         return SimpleVocabulary(series_terms)
>
>
> +class ArchiveArchVocabularyFactory:
> +    """A factory for generating vocabularies of an archive's arches in
> +    a given distroseries."""
> +
> +    implements(IContextSourceBinder)
> +
> +    def __call__(self, context):
> +        """Return a vocabulary created dynamically from the context archive.
> +
> +        :param context: The archive used to generating vocabulary.
> +        """
> +        arch_terms = []
> +        for arch in context.arches_with_binaries:
> +            term = SimpleTerm(
> +                arch, token=arch.architecturetag,
> +                title=arch.displayname)
> +            arch_terms.append(term)
> +        return SimpleVocabulary(arch_terms)
> +

Why is this a class and not a function? Is the implements() bit really
important?

> === added file 'lib/lp/soyuz/browser/tests/test_archive_view.py'
> --- lib/lp/soyuz/browser/tests/test_archive_view.py     1970-01-01 00:00:00 +0000
> +++ lib/lp/soyuz/browser/tests/test_archive_view.py     2010-03-18 09:24:24 +0000
> @@ -0,0 +1,59 @@
> +# Copyright 2010 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Test the main PPA (user) view functionality."""
> +
> +__metaclass__ = type
> +
> +import unittest
> +
> +from canonical.launchpad.ftests import login
> +from canonical.testing import LaunchpadFunctionalLayer
> +from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
> +from lp.testing import TestCaseWithFactory
> +from lp.testing.views import create_initialized_view
> +
> +
> +class TestArchiveView(TestCaseWithFactory):
> +
> +    layer = LaunchpadFunctionalLayer
> +

You need the librarian?

> +    def setUp(self)...

Read more...

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

On Thu, Mar 18, 2010 at 12:02 PM, Jonathan Lange <email address hidden> wrote:
...
>> +    def make_binaries_readable(self, binaries):
>
> Methods should be camelCase :( I hate that rule.
>

Per our recent discussion, this is comment is correct and the method
needs to change...

>> +    def test_batched_packages(self):
>
> test_batchedPackages :(
>

But this comment is not. The test can remain the same.

>> +    def test_batched_packages_filter_arch(self):
>
> testBatchedPackagesFilterArch
>

Likewise here.

jml

Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (4.2 KiB)

On Thu, Mar 18, 2010 at 12:03 PM, Jonathan Lange <email address hidden> wrote:
> On Thu, Mar 18, 2010 at 9:24 AM, Michael Nelson
 ...
>> === modified file 'lib/lp/soyuz/browser/archive.py'
>> --- lib/lp/soyuz/browser/archive.py     2010-03-10 12:50:18 +0000
>> +++ lib/lp/soyuz/browser/archive.py     2010-03-18 09:24:24 +0000
>> @@ -589,6 +589,26 @@
>>         return SimpleVocabulary(series_terms)
>>
>>
>> +class ArchiveArchVocabularyFactory:
>> +    """A factory for generating vocabularies of an archive's arches in
>> +    a given distroseries."""
>> +
>> +    implements(IContextSourceBinder)
>> +
>> +    def __call__(self, context):
>> +        """Return a vocabulary created dynamically from the context archive.
>> +
>> +        :param context: The archive used to generating vocabulary.
>> +        """
>> +        arch_terms = []
>> +        for arch in context.arches_with_binaries:
>> +            term = SimpleTerm(
>> +                arch, token=arch.architecturetag,
>> +                title=arch.displayname)
>> +            arch_terms.append(term)
>> +        return SimpleVocabulary(arch_terms)
>> +
>
> Why is this a class and not a function? Is the implements() bit really
> important?
>

Yep, but I used the zope.interface.directlyProvides as you mentioned,
and it worked well :)

>> === added file 'lib/lp/soyuz/browser/tests/test_archive_view.py'
>> --- lib/lp/soyuz/browser/tests/test_archive_view.py     1970-01-01 00:00:00 +0000
>> +++ lib/lp/soyuz/browser/tests/test_archive_view.py     2010-03-18 09:24:24 +0000
>> @@ -0,0 +1,59 @@
>> +# Copyright 2010 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Test the main PPA (user) view functionality."""
>> +
>> +__metaclass__ = type
>> +
>> +import unittest
>> +
>> +from canonical.launchpad.ftests import login
>> +from canonical.testing import LaunchpadFunctionalLayer
>> +from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
>> +from lp.testing import TestCaseWithFactory
>> +from lp.testing.views import create_initialized_view
>> +
>> +
>> +class TestArchiveView(TestCaseWithFactory):
>> +
>> +    layer = LaunchpadFunctionalLayer
>> +
>
> You need the librarian?

Yes, publishing test data with the SoyuzTestPublisher adds librarian
files for the tests.

>
>> +    def setUp(self):
>> +        """Create a ppa with some publishings for different arches
>> +        and in different distroseries."""
>
> Closing triple quotes on newline please.

Done.

>
>> +    def make_binaries_readable(self, binaries):
>
> Methods should be camelCase :( I hate that rule.

Done.

>
>> +    def test_batched_packages(self):
>
> test_batchedPackages :(

Not done, as your subsequent email.

>
>> +    def test_batched_packages_filter_arch(self):
>
> testBatchedPackagesFilterArch

And again.

>
>> === modified file 'lib/lp/soyuz/interfaces/archive.py'
>> --- lib/lp/soyuz/interfaces/archive.py  2010-03-17 16:45:38 +0000
>> +++ lib/lp/soyuz/interfaces/archive.py  2010-03-18 09:24:24 +0000
>> @@ -47,6 +47,7 @@
>>  from canonical.launchpad.interfaces.launchpad import IPrivacy
>>  from lp.registry.interfaces.role import IHasOwner
>> ...

Read more...

changes_jml.diff
1=== modified file 'lib/lp/soyuz/browser/archive.py'
2--- lib/lp/soyuz/browser/archive.py 2010-03-18 09:08:28 +0000
3+++ lib/lp/soyuz/browser/archive.py 2010-03-18 17:01:59 +0000
4@@ -33,7 +33,7 @@
5 from zope.app.form.browser import TextAreaWidget
6 from zope.component import getUtility
7 from zope.formlib import form
8-from zope.interface import implements, Interface
9+from zope.interface import directlyProvides, implements, Interface
10 from zope.security.proxy import removeSecurityProxy
11 from zope.schema import Choice, List, TextLine
12 from zope.schema.interfaces import IContextSourceBinder
13@@ -568,45 +568,22 @@
14 return list(copy_requests)
15
16
17-class ArchiveSeriesVocabularyFactory:
18- """A factory for generating vocabularies of an archive's series."""
19-
20- implements(IContextSourceBinder)
21-
22- def __call__(self, context):
23- """Return a vocabulary created dynamically from the context archive.
24-
25- :param context: The context used to generate the vocabulary. This
26- is passed automatically by the zope machinery. Therefore
27- this factory can only be used in a class where the context is
28- an IArchive.
29- """
30- series_terms = []
31- for distroseries in context.series_with_sources:
32- series_terms.append(
33- SimpleTerm(distroseries, token=distroseries.name,
34- title=distroseries.displayname))
35- return SimpleVocabulary(series_terms)
36-
37-
38-class ArchiveArchVocabularyFactory:
39- """A factory for generating vocabularies of an archive's arches in
40- a given distroseries."""
41-
42- implements(IContextSourceBinder)
43-
44- def __call__(self, context):
45- """Return a vocabulary created dynamically from the context archive.
46-
47- :param context: The archive used to generating vocabulary.
48- """
49- arch_terms = []
50- for arch in context.arches_with_binaries:
51- term = SimpleTerm(
52- arch, token=arch.architecturetag,
53- title=arch.displayname)
54- arch_terms.append(term)
55- return SimpleVocabulary(arch_terms)
56+def archive_series_vocabulary_factory(context):
57+ """Return a vocabulary created dynamically from the context archive.
58+
59+ :param context: The context used to generate the vocabulary. This
60+ is passed automatically by the zope machinery. Therefore
61+ this factory can only be used in a class where the context is
62+ an IArchive.
63+ """
64+ series_terms = []
65+ for distroseries in context.series_with_sources:
66+ series_terms.append(
67+ SimpleTerm(distroseries, token=distroseries.name,
68+ title=distroseries.displayname))
69+ return SimpleVocabulary(series_terms)
70+
71+directlyProvides(archive_series_vocabulary_factory, IContextSourceBinder)
72
73
74 class SeriesFilterWidget(LaunchpadDropdownWidget):
75@@ -625,7 +602,7 @@
76 title=_("Package name contains"), required=False)
77
78 series_filter = Choice(
79- source=ArchiveSeriesVocabularyFactory(), required=False)
80+ source=archive_series_vocabulary_factory, required=False)
81
82 status_filter = Choice(vocabulary=SimpleVocabulary((
83 SimpleTerm(active_publishing_status, 'published', 'Published'),
84@@ -633,10 +610,26 @@
85 )), required=False)
86
87
88+def archive_arch_vocabulary_factory(context):
89+ """Return a vocabulary created dynamically from the context archive.
90+
91+ :param context: The archive used to generating vocabulary.
92+ """
93+ arch_terms = []
94+ for arch in context.arches_with_binaries:
95+ term = SimpleTerm(
96+ arch, token=arch.architecturetag,
97+ title=arch.displayname)
98+ arch_terms.append(term)
99+ return SimpleVocabulary(arch_terms)
100+
101+directlyProvides(archive_arch_vocabulary_factory, IContextSourceBinder)
102+
103+
104 class IArchiveBinaryPackageFilter(IPPAPackageFilter):
105 """The interface used for filtering binary packages."""
106 arch_filter = Choice(
107- source=ArchiveArchVocabularyFactory(), required=False)
108+ source=archive_arch_vocabulary_factory, required=False)
109
110
111 class ArchivePackagesViewBase(ArchiveViewBase, LaunchpadFormView):
112
113=== modified file 'lib/lp/soyuz/browser/tests/test_archive_view.py'
114--- lib/lp/soyuz/browser/tests/test_archive_view.py 2010-03-18 09:06:00 +0000
115+++ lib/lp/soyuz/browser/tests/test_archive_view.py 2010-03-18 17:14:42 +0000
116@@ -29,7 +29,7 @@
117
118 test_publisher.getPubBinaries(archive=self.ppa)
119
120- def make_binaries_readable(self, binaries):
121+ def makeBinariesReadable(self, binaries):
122 """Return a string representation of the binaries."""
123 return ", ".join([
124 "%s in %s" % (
125@@ -45,7 +45,7 @@
126 self.assertEqual(
127 "foo-bin in ubuntutest Breezy Badger Autotest i386, "
128 "foo-bin in ubuntutest Breezy Badger Autotest hppa",
129- self.make_binaries_readable(view.batched_packages))
130+ self.makeBinariesReadable(view.batched_packages))
131
132 def test_batched_packages_filter_arch(self):
133 view = create_initialized_view(
134@@ -53,7 +53,7 @@
135 query_string="field.arch_filter=i386")
136 self.assertEqual(
137 "foo-bin in ubuntutest Breezy Badger Autotest i386",
138- self.make_binaries_readable(view.batched_packages))
139+ self.makeBinariesReadable(view.batched_packages))
140
141 def test_suite():
142 return unittest.TestLoader().loadTestsFromName(__name__)
lp://staging/~michael.nelson/launchpad/496862-ppa-installable-binaries updated
10544. By Michael Nelson

Refactored dynamic vocabularies to functions rather than classes.

10545. By Michael Nelson

test style method renames.

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

All good :)

review: Approve

Unmerged revisions

10545. By Michael Nelson

test style method renames.

10544. By Michael Nelson

Refactored dynamic vocabularies to functions rather than classes.

10543. By Michael Nelson

Removed branch todos as they're done.

10542. By Michael Nelson

Removed lint

10541. By Michael Nelson

Added unit tests for ArchiveView.batched_packages.

10540. By Michael Nelson

Added filtered binary packages to the ArchiveView.

10539. By Michael Nelson

Adds IArchive.arches_with_binaries.

10538. By Michael Nelson

Added failing test for IArchive.arches_with_binaries.

10537. By Michael Nelson

A few more s/sources/packages renaming.

10536. By Michael Nelson

Initial renaming to re-use ArchivePackagesViewBase for both source and binary
package listings.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.