launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+464639@xxxxxxxxxxxxxxxxxx>]

I updated the code - unfortunately once again without being able to run the tests.

It is impossible to apply any meaningful changes without a working test environment.

So we either need to merge this as is (after somebody has checked it out and ran the tests) and apply some changes later on (as adding more tests, refactor to an attribute), or somebody needs to pull these changes, use a cherry pick for the commit, and create a new merge proposal, so we have this merged today.

Diff comments:

> diff --git a/lib/lp/snappy/interfaces/snapbuild.py b/lib/lp/snappy/interfaces/snapbuild.py
> index 52019bf..46bd205 100644
> --- a/lib/lp/snappy/interfaces/snapbuild.py
> +++ b/lib/lp/snappy/interfaces/snapbuild.py
> @@ -370,6 +371,14 @@ class ISnapBuildView(IPackageBuildView, IPrivacy):
>  
>          :return: A collection of URLs for this build."""
>  
> +    @export_operation_as("build_metadata_url")

I did not know this is a thing. Is this somewhere documented?

I would suggest to get this merged as is, as I am currently not able to run tests. Then if you do not mind either you or Simone should follow up with a refactoring MP. The tests should still work as is.

> +    @export_read_operation()
> +    @operation_for_version("devel")
> +    def getBuildMetadataUrl():
> +        """URLs for all the files produced by this build.
> +
> +        :return: A collection of URLs for this build."""

Thanks!

> +
>  
>  class ISnapBuildEdit(IBuildFarmJobEdit):
>      """`ISnapBuild` attributes that require launchpad.Edit."""
> diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
> index df2e2c2..13a52b8 100644
> --- a/lib/lp/snappy/model/snapbuild.py
> +++ b/lib/lp/snappy/model/snapbuild.py
> @@ -425,6 +425,11 @@ class SnapBuild(PackageBuildMixin, StormBase):
>      def getFileUrls(self):
>          return [self.lfaUrl(lfa) for _, lfa, _ in self.getFiles()]
>  
> +    def getBuildMetadataUrl(self):
> +        for url in self.getFileUrls:

Thanks!

> Maybe it will be good return `None` if `use_fetch_service` is False?
I do not think this is necessary. There won't be a metadata, so it would automatically return None.

> +            if url.endswith("metadata.json"):

Makes sense! I added a comment.

> +                return url
> +
>      @cachedproperty
>      def eta(self):
>          """The datetime when the build job is estimated to complete.
> diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
> index d8b7758..edc917e 100644
> --- a/lib/lp/snappy/tests/test_snapbuild.py
> +++ b/lib/lp/snappy/tests/test_snapbuild.py
> @@ -1009,6 +1009,21 @@ class TestSnapBuildWebservice(TestCaseWithFactory):
>          for file_url in file_urls:
>              self.assertCanOpenRedirectedUrl(browser, file_url)
>  
> +    def test_getBuildMetadataUrl(self):

Unfortunately, I am running out of time, and without a working dev environment I cannot do this easily without breaking things again. But it makes certainly sense. I only created one test (blindly) to see whether things are working.

> +        # API clients can fetch the metadata from the build, generated by the
> +        # fetch service
> +        db_build = self.factory.makeSnapBuild(requester=self.person)
> +        metadata_file = self.factory.makeFileAlias(

Thanks!

> +            content="some_json",
> +            filename="xxx-metadata.json",

Thanks!

> +        )
> +        db_build.addFile(metadata_file)
> +
> +        build_url = api_url(db_build)
> +        logout()
> +        response = self.webservice.named_get(build_url, "build_metadata_url")
> +        self.assertEndsWith(response.jsonBody(), "metadata.json")
> +
>  
>  class TestSnapBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
>      """Test SnapBuild macaroon issuing and verification."""


-- 
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/464639
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:expose-build-metadata-via-api into launchpad:master.