launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+488673@xxxxxxxxxxxxxxxxxx>]

Thanks for reviewing! Addressed some of the comments, will finish it tomorrow.

Diff comments:

> diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
> index 1714f52..da41350 100644
> --- a/lib/lp/app/browser/tales.py
> +++ b/lib/lp/app/browser/tales.py
> @@ -733,6 +734,9 @@ class ObjectImageDisplayAPI:
>              sprite_string = "distribution"
>          elif IDistributionSourcePackage.providedBy(context):
>              sprite_string = "package-source"
> +        elif IExternalPackage.providedBy(context):
> +            # TODO: create a new sprite for ExternalPackages?

Adding name + date to this comment!

> +            sprite_string = "package-source"
>          elif ISprint.providedBy(context):
>              sprite_string = "meeting"
>          elif IBug.providedBy(context):
> diff --git a/lib/lp/app/widgets/tests/test_launchpadtarget.py b/lib/lp/app/widgets/tests/test_launchpadtarget.py
> index 40350b8..16f5f3e 100644
> --- a/lib/lp/app/widgets/tests/test_launchpadtarget.py
> +++ b/lib/lp/app/widgets/tests/test_launchpadtarget.py
> @@ -61,6 +61,9 @@ class LaunchpadTargetWidgetTestCase(TestCaseWithFactory):
>          self.package = self.factory.makeDSPCache(
>              distroseries=distroseries, sourcepackagename="snarf"
>          )
> +        self.externalpackage = self.factory.makeExternalPackage(
> +            distribution=self.distribution, sourcepackagename="snarf"

Done :)

> +        )
>          self.project = self.factory.makeProduct("pting")
>          field = Reference(__name__="target", schema=Interface, title="target")
>          field = field.bind(Thing())
> diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py
> index 8fcb135..69f7f0c 100644
> --- a/lib/lp/bugs/browser/bugtask.py
> +++ b/lib/lp/bugs/browser/bugtask.py
> @@ -320,7 +321,12 @@ class BugTargetTraversalMixin:
>          # rather than making it look as though this task was "not found",
>          # because it was filtered out by privacy-aware code.
>          for bugtask in bug.bugtasks:
> -            if bugtask.target == context:
> +            if bugtask.target == context or IExternalPackage.providedBy(

Agree, I did work on this for the next MP but I agree to change the workaround for this one!

I've changed it. I think we can live with this new workaround for now until I deploy the next MP as we don't have any way to create an ExternalPackage in production yet.

> +                bugtask.target
> +            ):
> +                # TODO: set +external urls for ExternalPackages
> +                # actually we select the first ExternalPackage that appears

Done!

> +
>                  # Security proxy this object on the way out.
>                  return getUtility(IBugTaskSet).get(bugtask.id)
>  
> diff --git a/lib/lp/bugs/browser/structuralsubscription.py b/lib/lp/bugs/browser/structuralsubscription.py
> index ec278f7..96f2687 100644
> --- a/lib/lp/bugs/browser/structuralsubscription.py
> +++ b/lib/lp/bugs/browser/structuralsubscription.py
> @@ -284,9 +285,11 @@ class StructuralSubscriptionView(LaunchpadFormView):
>      def userIsDriver(self):
>          """Has the current user driver permissions?"""
>          # We only want to look at this if the target is a
> -        # distribution source package, in order to maintain
> +        # distribution or external package, in order to maintain

done :)

>          # compatibility with the obsolete bug contacts feature.
> -        if IDistributionSourcePackage.providedBy(self.context):
> +        if IDistributionSourcePackage.providedBy(
> +            self.context
> +        ) or IExternalPackage.providedBy(self.context):
>              return check_permission(
>                  "launchpad.Driver", self.context.distribution
>              )
> diff --git a/lib/lp/bugs/model/bugtask.py b/lib/lp/bugs/model/bugtask.py
> index d6116ce..5e9af48 100644
> --- a/lib/lp/bugs/model/bugtask.py
> +++ b/lib/lp/bugs/model/bugtask.py
> @@ -371,6 +389,9 @@ def validate_target(
>                  )
>              except NotFoundError as e:
>                  raise IllegalTarget(e.args[0])
> +    elif IExternalPackage.providedBy(target):
> +        # TODO: Check with store/soss that package exists

done :)

> +        pass
>  
>      legal_types = target.pillar.getAllowedBugInformationTypes()
>      new_pillar = target.pillar not in bug.affected_pillars
> diff --git a/lib/lp/bugs/model/structuralsubscription.py b/lib/lp/bugs/model/structuralsubscription.py
> index 37e8be2..debb06e 100644
> --- a/lib/lp/bugs/model/structuralsubscription.py
> +++ b/lib/lp/bugs/model/structuralsubscription.py
> @@ -607,7 +608,12 @@ def get_structural_subscriptions_for_bug(bug, person=None):
>      # This is here because of a circular import.
>      from lp.registry.model.person import Person
>  
> -    bugtasks = bug.bugtasks
> +    bugtasks = []
> +    # TODO: support bug subscriptions

done

> +    for bugtask in bug.bugtasks:
> +        if not IExternalPackage.providedBy(bugtask.target):
> +            bugtasks.append(bugtask)
> +
>      if not bugtasks:
>          return EmptyResultSet()
>      conditions = []
> diff --git a/lib/lp/bugs/tests/test_structuralsubscription.py b/lib/lp/bugs/tests/test_structuralsubscription.py
> index ce1f712..a22e0d6 100644
> --- a/lib/lp/bugs/tests/test_structuralsubscription.py
> +++ b/lib/lp/bugs/tests/test_structuralsubscription.py
> @@ -548,6 +548,22 @@ class TestGetStructuralSubscriptionTargets(TestCaseWithFactory):
>              },
>          )
>  
> +    def test_externalpackage_target(self):
> +        actor = self.factory.makePerson()
> +        login_person(actor)
> +        externalpackage = self.factory.makeExternalPackage()
> +        product = self.factory.makeProduct()
> +        bug = self.factory.makeBug(target=product)
> +        bug.addTask(actor, externalpackage)
> +        product_bugtask = bug.bugtasks[0]
> +        result = get_structural_subscription_targets(bug.bugtasks)
> +        self.assertEqual(
> +            set(result),
> +            {
> +                (product_bugtask, product),

good catch, done!

> +            },
> +        )
> +
>      def test_product_with_project_group(self):
>          # get_structural_subscription_targets() will yield both a
>          # product and its parent project group if it has one.
> diff --git a/lib/lp/registry/interfaces/externalpackage.py b/lib/lp/registry/interfaces/externalpackage.py
> new file mode 100644
> index 0000000..f2dccf9
> --- /dev/null
> +++ b/lib/lp/registry/interfaces/externalpackage.py
> @@ -0,0 +1,148 @@
> +# Copyright 2009, 2025 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""External package interfaces."""
> +
> +__all__ = [
> +    "IExternalPackage",
> +    "ExternalPackageType",
> +]
> +
> +from lazr.enum import DBEnumeratedType, DBItem
> +from lazr.restful.declarations import exported, exported_as_webservice_entry
> +from lazr.restful.fields import Reference
> +from zope.interface import Attribute
> +from zope.schema import TextLine
> +
> +from lp import _
> +from lp.app.interfaces.launchpad import IHeadingContext
> +from lp.bugs.interfaces.bugtarget import IBugTarget, IHasOfficialBugTags
> +from lp.registry.interfaces.distribution import IDistribution
> +from lp.registry.interfaces.role import IHasDrivers
> +
> +
> +@exported_as_webservice_entry(as_of="beta")
> +class IExternalPackageView(
> +    IHeadingContext,
> +    IBugTarget,
> +    IHasOfficialBugTags,
> +    IHasDrivers,
> +):
> +    """`IExternalPackage` attributes that require launchpad.View."""
> +
> +    packagetype = Attribute("The package type")
> +
> +    channel = Attribute("The package channel")
> +
> +    display_channel = TextLine(title=_("Display channel name"), readonly=True)
> +
> +    distribution = exported(
> +        Reference(IDistribution, title=_("The distribution."))
> +    )
> +    sourcepackagename = Attribute("The source package name.")
> +
> +    name = exported(
> +        TextLine(title=_("The source package name as text"), readonly=True)
> +    )
> +    display_name = exported(
> +        TextLine(title=_("Display name for this package."), readonly=True)
> +    )
> +    displayname = Attribute("Display name (deprecated)")
> +    title = exported(
> +        TextLine(title=_("Title for this package."), readonly=True)
> +    )
> +
> +    drivers = Attribute("The drivers for the distribution.")
> +
> +    def __eq__(other):
> +        """IExternalPackage comparison method.
> +
> +        Distro sourcepackages compare equal only if their fields compare equal.
> +        """
> +
> +    def __ne__(other):
> +        """IExternalPackage comparison method.
> +
> +        External packages compare not equal if either of their
> +        fields compare not equal.
> +        """
> +
> +
> +@exported_as_webservice_entry(as_of="beta")
> +class IExternalPackage(
> +    IExternalPackageView,
> +):
> +    """Represents an ExternalPackage in a distribution.
> +
> +    Create IExternalPackage by invoking `IDistribution.getExternalPackage()`.
> +    """
> +
> +
> +class ExternalPackageType(DBEnumeratedType):
> +    """Bug Task Status
> +
> +    The various possible states for a bugfix in a specific place.
> +    """
> +
> +    SNAP = DBItem(

I would say that we will need the Unknown package type = 0, so I'll add that one

> +        1,
> +        """
> +        Snap
> +
> +        Snap external package
> +        """,
> +    )
> +
> +    CHARM = DBItem(
> +        2,
> +        """
> +        Charm
> +
> +        Charm external package
> +        """,
> +    )
> +
> +    ROCK = DBItem(
> +        3,
> +        """
> +        Rock
> +
> +        Rock external package
> +        """,
> +    )
> +
> +    PYTHON = DBItem(
> +        4,
> +        """
> +        Python
> +
> +        Python external package
> +        """,
> +    )
> +
> +    CONDA = DBItem(
> +        5,
> +        """
> +        Conda
> +
> +        Conda external package
> +        """,
> +    )
> +
> +    CARGO = DBItem(
> +        6,
> +        """
> +        Cargo
> +
> +        Cargo external package
> +        """,
> +    )
> +
> +    MAVEN = DBItem(
> +        7,
> +        """
> +        Maven
> +
> +        Maven external package
> +        """,
> +    )


-- 
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/488673
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-external-package into launchpad:master.