launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Diff comments:

> === modified file 'lib/lp/snappy/model/snapbuildjob.py'
> --- lib/lp/snappy/model/snapbuildjob.py	2018-12-18 18:10:39 +0000
> +++ lib/lp/snappy/model/snapbuildjob.py	2019-06-19 10:53:29 +0000
> @@ -350,7 +350,6 @@
>                      raise ManualReview(
>                          "Package held for manual review on the store; "
>                          "cannot release it automatically.")

Agree that we can't get the manual review check now, so removed that.
Altered the checkStatus to not check can_release, but made retrieving the revision optional via a `.get`, as I think if the upload does get held for manual release it won't have a revision (although we don't care here that it has been).

> -                client.release(self.snapbuild, self.store_revision)

Done, I think I got it all.

>              self.error_message = None
>          except self.retry_error_types:
>              raise
> 
> === modified file 'lib/lp/snappy/model/snapstoreclient.py'
> --- lib/lp/snappy/model/snapstoreclient.py	2019-06-14 14:46:15 +0000
> +++ lib/lp/snappy/model/snapstoreclient.py	2019-06-19 10:53:29 +0000
> @@ -266,6 +266,14 @@
>              "series": snap.store_series.name,
>              "built_at": snapbuild.date_started.isoformat(),
>              }
> +        # The security proxy is useless and breaks JSON serialisation.
> +        channels = removeSecurityProxy(snap.store_channels)
> +        if channels:
> +            # This will cause a release
> +            data.update({
> +                "channels": channels,
> +                "only_if_newer": True,
> +            })

Fixed. One day I'll get used to this?

>          # XXX cjwatson 2016-05-09: This should add timeline information, but
>          # that's currently difficult in jobs.
>          try:
> 
> === modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
> --- lib/lp/snappy/tests/test_snapstoreclient.py	2019-06-14 14:49:25 +0000
> +++ lib/lp/snappy/tests/test_snapstoreclient.py	2019-06-19 10:53:29 +0000
> @@ -638,100 +673,3 @@
>          self.assertContentEqual([], responses.calls)
>          self.assertIsNone(
>              getUtility(IMemcacheClient).get(self.channels_memcache_key))
> -
> -    @responses.activate
> -    def test_release(self):
> -        snap = self.factory.makeSnap(
> -            store_upload=True,
> -            store_series=self.factory.makeSnappySeries(name="rolling"),
> -            store_name="test-snap",
> -            store_secrets=self._make_store_secrets(),
> -            store_channels=["stable", "edge"])
> -        snapbuild = self.factory.makeSnapBuild(snap=snap)
> -        self._addSnapReleaseResponse()

Fixed, also removed import

> -        self.client.release(snapbuild, 1)
> -        self.assertThat(responses.calls[-1].request, RequestMatches(
> -            url=Equals("http://sca.example/dev/api/snap-release/";),
> -            method=Equals("POST"),
> -            headers=ContainsDict({"Content-Type": Equals("application/json")}),
> -            auth=("Macaroon", MacaroonsVerify(self.root_key)),
> -            json_data={
> -                "name": "test-snap", "revision": 1,
> -                "channels": ["stable", "edge"], "series": "rolling",
> -                }))
> -
> -    @responses.activate
> -    def test_release_no_discharge(self):
> -        root_key = hashlib.sha256(self.factory.getUniqueString()).hexdigest()
> -        root_macaroon = Macaroon(key=root_key)
> -        snap = self.factory.makeSnap(
> -            store_upload=True,
> -            store_series=self.factory.makeSnappySeries(name="rolling"),
> -            store_name="test-snap",
> -            store_secrets={"root": root_macaroon.serialize()},
> -            store_channels=["stable", "edge"])
> -        snapbuild = self.factory.makeSnapBuild(snap=snap)
> -        self._addSnapReleaseResponse()
> -        self.client.release(snapbuild, 1)
> -        self.assertThat(responses.calls[-1].request, RequestMatches(
> -            url=Equals("http://sca.example/dev/api/snap-release/";),
> -            method=Equals("POST"),
> -            headers=ContainsDict({"Content-Type": Equals("application/json")}),
> -            auth=("Macaroon", MacaroonsVerify(root_key)),
> -            json_data={
> -                "name": "test-snap", "revision": 1,
> -                "channels": ["stable", "edge"], "series": "rolling",
> -                }))
> -
> -    @responses.activate
> -    def test_release_needs_discharge_macaroon_refresh(self):
> -        store_secrets = self._make_store_secrets()
> -        snap = self.factory.makeSnap(
> -            store_upload=True,
> -            store_series=self.factory.makeSnappySeries(name="rolling"),
> -            store_name="test-snap", store_secrets=store_secrets,
> -            store_channels=["stable", "edge"])
> -        snapbuild = self.factory.makeSnapBuild(snap=snap)
> -        responses.add(
> -            "POST", "http://sca.example/dev/api/snap-release/";, status=401,
> -            headers={"WWW-Authenticate": "Macaroon needs_refresh=1"})
> -        self._addMacaroonRefreshResponse()
> -        self._addSnapReleaseResponse()
> -        self.client.release(snapbuild, 1)
> -        requests = [call.request for call in responses.calls]
> -        self.assertThat(requests, MatchesListwise([
> -            MatchesStructure.byEquality(path_url="/dev/api/snap-release/"),
> -            MatchesStructure.byEquality(path_url="/api/v2/tokens/refresh"),
> -            MatchesStructure.byEquality(path_url="/dev/api/snap-release/"),
> -            ]))
> -        self.assertNotEqual(
> -            store_secrets["discharge"], snap.store_secrets["discharge"])
> -
> -    @responses.activate
> -    def test_release_error(self):
> -        snap = self.factory.makeSnap(
> -            store_upload=True,
> -            store_series=self.factory.makeSnappySeries(name="rolling"),
> -            store_name="test-snap", store_secrets=self._make_store_secrets(),
> -            store_channels=["stable", "edge"])
> -        snapbuild = self.factory.makeSnapBuild(snap=snap)
> -        responses.add(
> -            "POST", "http://sca.example/dev/api/snap-release/";, status=503,
> -            json={"error_list": [{"message": "Failed to publish"}]})
> -        self.assertRaisesWithContent(
> -            ReleaseFailedResponse, "Failed to publish",
> -            self.client.release, snapbuild, 1)
> -
> -    @responses.activate
> -    def test_release_404(self):
> -        snap = self.factory.makeSnap(
> -            store_upload=True,
> -            store_series=self.factory.makeSnappySeries(name="rolling"),
> -            store_name="test-snap", store_secrets=self._make_store_secrets(),
> -            store_channels=["stable", "edge"])
> -        snapbuild = self.factory.makeSnapBuild(snap=snap)
> -        responses.add(
> -            "POST", "http://sca.example/dev/api/snap-release/";, status=404)
> -        self.assertRaisesWithContent(
> -            ReleaseFailedResponse, b"404 Client Error: Not Found",
> -            self.client.release, snapbuild, 1)


-- 
https://code.launchpad.net/~twom/launchpad/snap-release-intents/+merge/369028
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.