[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into 
launchpad:master has been updated.

Description changed to:

Tests from CIBuilds that tested the redacted secrets still ran successfully.

No other build type has `secrets` in their args except for snap builds and ci 
builds, but IMO anything that goes under a "secrets" keywords should be 
redacted - so I updated it within the parent `redactXmlrpcArguments` method.

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465326
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into 
launchpad:master.

Commit message:
Redact `fetch_service_mitm_certificate` in build logs

When running a fetch service build, we send the certficate from buildd-manager 
to buildd, and log it. This redacts the certificate.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465326
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-redact-certificate-in-logs into 
launchpad:master.
diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
index e8d82d2..4dd258a 100644
--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
@@ -12,6 +12,7 @@ import logging
 import os
 import tempfile
 from collections import OrderedDict
+from copy import deepcopy
 from datetime import datetime
 
 import transaction
@@ -126,6 +127,14 @@ class BuildFarmJobBehaviourBase:
 
 def redactXmlrpcArguments(self, args):
 # we do not want to have secrets in logs
+
+# we need to copy the input in order to avoid mutating `args` which
+# will be passed to the builders
+args = deepcopy(args)
+if args["args"].get("secrets"):
+for key in args["args"]["secrets"].keys():
+args["args"]["secrets"][key] = ""
+
 return sanitise_urls(repr(args))
 
 @defer.inlineCallbacks
diff --git a/lib/lp/code/model/cibuildbehaviour.py b/lib/lp/code/model/cibuildbehaviour.py
index 6cceb72..ba73848 100644
--- a/lib/lp/code/model/cibuildbehaviour.py
+++ b/lib/lp/code/model/cibuildbehaviour.py
@@ -9,7 +9,6 @@ __all__ = [
 
 import json
 from configparser import NoSectionError
-from copy import deepcopy
 from typing import Any, Generator
 
 from twisted.internet import defer
@@ -119,17 +118,6 @@ class CIBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
 
 ALLOWED_STATUS_NOTIFICATIONS = ["PACKAGEFAIL"]
 
-def redactXmlrpcArguments(self, args):
-# we do not want to have secrets in logs
-
-# we need to copy the input in order to avoid mutating `args` which
-# will be passed to the builders
-args = deepcopy(args)
-if args["args"].get("secrets"):
-for key in args["args"]["secrets"].keys():
-args["args"]["secrets"][key] = ""
-return super().redactXmlrpcArguments(args)
-
 def getLogFileName(self):
 return "buildlog_ci_%s_%s_%s.txt" % (
 self.build.git_repository.name,
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index cb2269b..66fb638 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -419,6 +419,45 @@ class TestAsyncSnapBuildBehaviourFetchService(
 self.assertEqual([], self.fetch_service_api.sessions.requests)
 
 @defer.inlineCallbacks
+def test_requestFetchServiceSession_mitm_certficate_redacted(self):
+"""The `fetch_service_mitm_certificate` field in the build arguments
+is redacted in the build logs."""
+
+self.useFixture(
+FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
+)
+snap = self.factory.makeSnap(use_fetch_service=True)
+request = self.factory.makeSnapBuildRequest(snap=snap)
+job = self.makeJob(snap=snap, build_request=request)
+args = yield job.extraBuildArgs()
+
+chroot_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+job.build.distro_arch_series.addOrUpdateChroot(
+chroot_lfa, image_type=BuildBaseImageType.CHROOT
+)
+lxd_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+job.build.distro_arch_series.addOrUpdateChroot(
+lxd_lfa, image_type=BuildBaseImageType.LXD
+)
+deferred = defer.Deferred()
+deferred.callback(None)
+job._worker.sendFileToWorker = MagicMock(return_value=deferred)
+job._worker.build = MagicMock(return_value=(None, None))
+
+logger = BufferLogger()
+yield job.dispatchBuildToWorker(logger)
+
+# Secrets exist within the arguments
+self.assertIn(
+"fake-cert", args["secrets"]["fetch_service_mitm_certificate"]
+)
+# But are redacted in the log output
+self.assertIn(
+"'fetch_service_mitm_certificate': ''",
+logger.getLogBuffer(),
+)
+
+@defer.inlineCallbacks
 def te

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update into ~launchpad/launchpad-mojo-specs/+git/private:vbuilder

[Ines Almeida]

Ideally what should happen when deploying is that we either: (1) should only 
upgrade all the builders if all images exist (hard to implement), or we upgrade 
all of the images that can be upgraded and investigate and fix the one(s) that 
failed (what this MP does).

Regardless of the reason why images are missing, the current behavior is far 
from ideal - we neither refrain from deploying, nor do we deploy all but the 1 
failing. Moving forward with the deployment and listing the images that failed 
with a big THESE ONES FAILED warning, is much better here.

---

Regarding the why images were missing in qastaging: some 
`glance-simplestreams-sync` units (that build the images) were deactivated in 
qastaging due to network issues from last year. I had a chat with Colin about 
it. A lot of our builders in qastaging are not in a clear state, and we need to 
address it.

But note that currently it is NOT POSSIBLE to deploy buildd to qastaging for 
the builders that are active and working (it is only possible now because I 
cowboyed this update there), and it wouldn't be possible to know which 
arch/regions are failing because it fails in the first iteration of the 
deployment.

This change lists the failing units, so that we can address them all, and still 
move forward in our projects.

---

I updated the error message to be more pressing that something needs to be 
addressed (just made it all Caps with the word `FAILURE` on it), and updated 
the naming of `instanced` to `applications` to make it coherent.

---

As mentioned before in the description, after updating this, we will also need 
to update the `buildd` deployment docs to mention addressing this error message 
if it appears.

Diff comments:

> diff --git a/vbuilder/rebuild-images b/vbuilder/rebuild-images
> index b4f8348..b28fc02 100755
> --- a/vbuilder/rebuild-images
> +++ b/vbuilder/rebuild-images
> @@ -77,8 +78,17 @@ def main():
>  "juju", "ssh", unit, "sudo", 
> "/usr/local/bin/rebuild-latest-image",
>  f"{name_prefix}/ubuntu-{series}-daily-{arch}-",
>  ]
> -utils.run(None, rebuild_cmd)
> +try:
> +utils.run(None, rebuild_cmd)
> +except Exception as e:

The exception raised is actually a generic `Exception`.

```
Exception: No active image starting with 
'launchpad-buildd-qastaging/ubuntu-focal-daily-arm64-gpu-'
```

> +failed_target_instances.append(application)
> +print(f"Command in `{application}` failed with error: {str(e)}")
>  
> +print("\n\nRun completed.")
> +if failed_target_instances:
> +print("Rebuilding images failed in the following instances:")
> +for instance in failed_target_instances:
> +print(f" - {instance}")
>  
>  if __name__ == "__main__":
>  main()


-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/464936
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:vbuilder.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master

[Ines Almeida]

Fixed the mypy issues and updated the logic to use the `use_fetch_service` flag 
directly from the builder proxy info. I still need to re-work a test, but this 
is now ready for review.
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465152
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:release-237 into launchpad-buildd:master

[Ines Almeida]

Ines Almeida has proposed merging ~ines-almeida/launchpad-buildd:release-237 
into launchpad-buildd:master.

Commit message:
Release version 237

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/465241
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:release-237 into launchpad-buildd:master.
diff --git a/debian/changelog b/debian/changelog
index f261f59..17ed866 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,9 +1,18 @@
-launchpad-buildd (237) UNRELEASED; urgency=medium
+launchpad-buildd (237) focal; urgency=medium
 
   [ Simone Pelosi ]
   * Improve documentation for qastaging deployment.
 
- -- Simone Pelosi   Thu, 22 Feb 2024 17:16:55 +0100
+  [ Inês Almeida ]
+  * Add logic to allow using fetch-service as the builder proxy for snaps that
+have the `use_fetch_service` flag on.
+Update token revocation authentication when using the fetch service.
+Install mitm-certificates that are now injected from the buildd-manager.
+  * Improve documentation for qastaging deployment.
+  * Add `proxy_info()` xmlrpc endpoint that can be used to retrieve proxy
+details from a builder.
+
+ -- Inês Almeida   Wed, 24 Apr 2024 13:20:40 +0200
 
 launchpad-buildd (236) focal; urgency=medium
 
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master

[Ines Almeida]

This has not been cleaned up (and mypy issues not fixed). Opening it up for an 
early review.
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465152
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master.

Commit message:
Update how to keep track of session_id within a builder session

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465152
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-end-session into launchpad:master.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 1fc0d9b..88f1792 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -17,6 +17,7 @@ __all__ = [
 
 import base64
 import os
+import re
 import time
 from typing import Dict, Generator, Type
 
@@ -56,11 +57,6 @@ class BuilderProxyMixin:
 build: BuildFarmJob
 _worker: BuilderWorker
 
-def __init__(self, *args, **kwargs):
-super().__init__(*args, **kwargs)
-self._use_fetch_service = False
-self._proxy_service = None
-
 @defer.inlineCallbacks
 def startProxySession(
 self,
@@ -69,15 +65,15 @@ class BuilderProxyMixin:
 use_fetch_service: bool = False,
 ) -> Generator[None, Dict[str, str], None]:
 
-self._use_fetch_service = use_fetch_service
-
 if not allow_internet:
 return
 
 if not use_fetch_service and _get_proxy_config("builder_proxy_host"):
-proxy_service: Type[IProxyService] = BuilderProxy
+proxy_service: Type[IProxyService] = BuilderProxy(
+worker=self._worker
+)
 elif use_fetch_service and _get_proxy_config("fetch_service_host"):
-proxy_service = FetchService
+proxy_service = FetchService(worker=self._worker)
 
 # Append the fetch-service certificate to BuildArgs secrets.
 if "secrets" not in args:
@@ -91,17 +87,16 @@ class BuilderProxyMixin:
 # non-production environments.
 return
 
-self._proxy_service = proxy_service(
-build_id=self.build.build_cookie, worker=self._worker
+session_data = yield proxy_service.startSession(
+build_id=self.build.build_cookie
 )
-session_data = yield self._proxy_service.startSession()
 
 args["proxy_url"] = session_data["proxy_url"]
 args["revocation_endpoint"] = session_data["revocation_endpoint"]
 args["use_fetch_service"] = use_fetch_service
 
 @defer.inlineCallbacks
-def endProxySession(self, upload_path: str):
+def endProxySession(self, upload_path: str, use_fetch_service: bool):
 """Handles all the necessary cleanup to be done at the end of a build.
 
 For the fetch service case, this means:
@@ -115,30 +110,32 @@ class BuilderProxyMixin:
 Sessions will be closed automatically within the Fetch Service after
 a certain amount of time configured by its charm (default 6 hours).
 """
-if not self._use_fetch_service:
+if not use_fetch_service:
 # No cleanup needed for the builder proxy
 return
 
-if self._proxy_service is None:
-# A session was never started. This can happen if the proxy configs
-# are not set (see `startProxySession`)
-return
+proxy_service = FetchService(worker=self._worker)
+proxy_info = yield self._worker.proxy_info()
+session_id = proxy_service.parseRevocationEndpoint(
+proxy_info["revocation_endpoint"]
+)
 
 metadata_file_name = BUILD_METADATA_FILENAME_FORMAT.format(
 build_id=self.build.build_cookie
 )
 file_path = os.path.join(upload_path, metadata_file_name)
-yield self._proxy_service.retrieveMetadataFromSession(
-save_content_to=file_path
+yield proxy_service.retrieveMetadataFromSession(
+session_id=session_id,
+save_content_to=file_path,
 )
 
-yield self._proxy_service.endSession()
+yield proxy_service.endSession(session_id=session_id)
 
 
 class IProxyService:
 """Interface for Proxy Services - either FetchService or BuilderProxy."""
 
-def __init__(self, build_id: str, worker: BuilderWorker):
+def __init__(self, worker: BuilderWorker):
 pass
 
 @defer.inlineCallbacks
@@ -159,7 +156,7 @@ class BuilderProxy(IProxyService):
 making API requests directly to the builder proxy control endpoint.
 """
 
-def __init__(self, build_id: str, worker: BuilderWorker):
+def __init__(self, worker: BuilderWorker):
 self.control_endpoint = _get_value_from_config(
 

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:fetch-service-add-proxy-info-endpoint into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:fetch-service-add-proxy-info-endpoint into 
launchpad-buildd:master has been updated.

Commit message changed to:

Add `proxy_info` endpoint that returns details for the proxy setup

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/465154
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:fetch-service-add-proxy-info-endpoint into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:fetch-service-add-proxy-info-endpoint into launchpad-buildd:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-buildd:fetch-service-add-proxy-info-endpoint into 
launchpad-buildd:master.

Commit message:
WIP

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/465154
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:fetch-service-add-proxy-info-endpoint into 
launchpad-buildd:master.
diff --git a/bin/test_buildd_generatetranslationtemplates b/bin/test_buildd_generatetranslationtemplates
index ffaf819..be4e1ef 100755
--- a/bin/test_buildd_generatetranslationtemplates
+++ b/bin/test_buildd_generatetranslationtemplates
@@ -19,6 +19,7 @@ chroot_sha1 = sys.argv[1]
 
 proxy = ServerProxy("http://localhost:8221/rpc;)
 print(proxy.info())
+print(proxy.proxy_info())
 print(proxy.status())
 buildid = "1-2"
 build_type = "translation-templates"
diff --git a/bin/test_buildd_recipe b/bin/test_buildd_recipe
index a1b4a26..4622396 100755
--- a/bin/test_buildd_recipe
+++ b/bin/test_buildd_recipe
@@ -25,6 +25,7 @@ def deb_line(host, suites):
 proxy = ServerProxy("http://localhost:8221/rpc;)
 print(proxy.echo("Hello World"))
 print(proxy.info())
+print(proxy.proxy_info())
 status = proxy.status()
 print(status)
 if status[0] != "BuilderStatus.IDLE":
diff --git a/lpbuildd/builder.py b/lpbuildd/builder.py
index ed624b3..f821b34 100644
--- a/lpbuildd/builder.py
+++ b/lpbuildd/builder.py
@@ -786,6 +786,11 @@ class XMLRPCBuilder(xmlrpc.XMLRPC):
 """Echo the argument back."""
 return args
 
+def xmlrpc_proxy_info(self):
+"""Return the details for the proxy used by the manager."""
+proxy_fields = ["use_fetch_service", "revocation_endpoint"]
+return {k: getattr(self.builder.manager, k) for k in proxy_fields}
+
 def xmlrpc_info(self):
 """Return the protocol version and the manager methods supported."""
 return (
diff --git a/lpbuildd/tests/test_buildd.py b/lpbuildd/tests/test_buildd.py
index 569a901..616fd9d 100644
--- a/lpbuildd/tests/test_buildd.py
+++ b/lpbuildd/tests/test_buildd.py
@@ -253,3 +253,9 @@ class XMLRPCBuilderTests(unittest.TestCase):
 info.startswith("%s not in [" % buildername),
 'UNKNOWNBUILDER info is "%s"' % info,
 )
+
+def test_proxy_info(self):
+"""Check that `proxy_info` returns expected details from the proxy."""
+proxy_info = self.server.proxy_info()
+self.assertIn("revocation_endpoint", proxy_info)
+self.assertIn("use_fetch_service", proxy_info)
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-add-missing-fetch-service-args into launchpad:master

[Ines Almeida]

Updated (still squashed into 1 commit since it was just a 1-liner)

This was a very quick MP that I opened right before the EOD to get it out of 
the way, it isn't blocking any testingas I cowboyed it in qastaging.

As fornoticing the missing arg, I simply noticed some code wasn't running in 
buildd, and I checked the build logs to find that this expected arg wasn't 
there. Adding it fixed the initial issue.
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465003
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-add-missing-fetch-service-args into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-add-missing-fetch-service-args into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-add-missing-fetch-service-args into 
launchpad:master.

Commit message:
Add missing `use_fetch_service` arg when starting a build

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/465003
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-add-missing-fetch-service-args into 
launchpad:master.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 9bacf76..1fc0d9b 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -98,6 +98,7 @@ class BuilderProxyMixin:
 
 args["proxy_url"] = session_data["proxy_url"]
 args["revocation_endpoint"] = session_data["revocation_endpoint"]
+args["use_fetch_service"] = use_fetch_service
 
 @defer.inlineCallbacks
 def endProxySession(self, upload_path: str):
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update into ~launchpad/launchpad-mojo-specs/+git/private:vbuilder

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update
 into ~launchpad/launchpad-mojo-specs/+git/private:vbuilder has been updated.

Description changed to:

For context, this is a script that runs as part of the deployment of 
launchpad-buildd (see 
https://launchpad-buildd.readthedocs.io/en/latest/how-to/deployment.html).

I will also do some changes to the deployment documentation in the 
`launchpad-buildd` repository as well, to add more context.

As it was, one image missing in qastaging would lead to not being able to 
deploy to any of the other builders. This will make deployment clearer by 
letting us know which instances had issues out of all of them, and unblocking 
deployments when there are known issues with a certain image.

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/464936
-- 
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:vbuilder.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update into launchpad-mojo-specs:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update
 into launchpad-mojo-specs:master has been updated.

Status: Work in progress => Superseded

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/464935
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update into ~launchpad/launchpad-mojo-specs/+git/private:vbuilder

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:vbuilder-rebuild-images-manifest-update
 into ~launchpad/launchpad-mojo-specs/+git/private:vbuilder.

Commit message:
Update `rebuild-images` script to be more resilient when images are missing

The script now skips the instances with the missing images, and log it in a 
message at the end of the run.
This is important because we might have known issues with building some images.


Requested reviews:
  Canonical Launchpad Engineering (launchpad)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/464936

I will also do some changes to the deployment documentation in the 
`launchpad-buildd` repository as well, to add more context.

As it was, one image missing in qastaging would lead to not being able to 
deploy to any of the other builders. This will make deployment clearer by 
letting us know which instances had issues out of all of them, and unblocking 
deployments when there are known issues with a certain image.
-- 
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:vbuilder.
diff --git a/vbuilder/rebuild-images b/vbuilder/rebuild-images
index b4f8348..b28fc02 100755
--- a/vbuilder/rebuild-images
+++ b/vbuilder/rebuild-images
@@ -70,6 +70,7 @@ def main():
 name_prefix = name_prefix_by_stage[stage]
 targets = targets_by_stage[stage]
 juju_services = utils.juju_services()
+failed_target_instances = []
 for region, arch, series in targets:
 application = f"glance-simplestreams-sync-{region}-{arch}"
 unit = get_leader_unit(juju_services, application)
@@ -77,8 +78,17 @@ def main():
 "juju", "ssh", unit, "sudo", "/usr/local/bin/rebuild-latest-image",
 f"{name_prefix}/ubuntu-{series}-daily-{arch}-",
 ]
-utils.run(None, rebuild_cmd)
+try:
+utils.run(None, rebuild_cmd)
+except Exception as e:
+failed_target_instances.append(application)
+print(f"Command in `{application}` failed with error: {str(e)}")
 
+print("\n\nRun completed.")
+if failed_target_instances:
+print("Rebuilding images failed in the following instances:")
+for instance in failed_target_instances:
+print(f" - {instance}")
 
 if __name__ == "__main__":
 main()
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master.

Commit message:
Add DB load bulk load for `build_metadata_url` attributes when getting multiple 
builds

This reduces the number of DB queries made in the case for where a snap has 
multiple builds.
Also: revert commit that commented out a test that verified the snap.builds 
queries number



Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464839

The test that was previously commented out now runs successfully.
Also added a new test, and re-ran the old `build_metadata_url` related tests 
succesfully.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master.
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index ea7bf99..842b142 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -6,6 +6,7 @@ __all__ = [
 "SnapFile",
 ]
 
+from collections import defaultdict
 from datetime import timedelta, timezone
 from operator import attrgetter
 
@@ -51,7 +52,7 @@ from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.person import Person
 from lp.services.config import config
-from lp.services.database.bulk import load_related
+from lp.services.database.bulk import load_referencing, load_related
 from lp.services.database.constants import DEFAULT
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
@@ -427,12 +428,15 @@ class SnapBuild(PackageBuildMixin, StormBase):
 return [self.lfaUrl(lfa) for _, lfa, _ in self.getFiles()]
 
 @property
-def build_metadata_url(self):
-metadata_filename = BUILD_METADATA_FILENAME_FORMAT.format(
+def metadata_filename(self):
+return BUILD_METADATA_FILENAME_FORMAT.format(
 build_id=self.build_cookie
 )
+
+@cachedproperty
+def build_metadata_url(self):
 try:
-return self.lfaUrl(self.getFileByName(metadata_filename))
+return self.lfaUrl(self.getFileByName(self.metadata_filename))
 except NotFoundError:
 return None
 
@@ -663,6 +667,24 @@ class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
 )
 load_related(Job, sbjs, ["job_id"])
 
+# Prefetch all snaps metadata files
+snap_files = load_referencing(SnapFile, builds, ["snapbuild_id"])
+lfas = load_related(LibraryFileAlias, snap_files, ["libraryfile_id"])
+
+metadata_files = defaultdict(list)
+for snap_file in snap_files:
+if (
+snap_file.libraryfile.filename
+== snap_file.snapbuild.metadata_filename
+):
+metadata_files[snap_file.snapbuild_id] = snap_file.libraryfile
+
+for build in builds:
+cache = get_property_cache(build)
+cache.build_metadata_url = build.lfaUrl(
+metadata_files.get(build.id)
+)
+
 def getByBuildFarmJobs(self, build_farm_jobs):
 """See `ISpecificBuildFarmJobSource`."""
 if len(build_farm_jobs) == 0:
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index eace8dd..7aa8c29 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -5798,70 +5798,121 @@ class TestSnapWebservice(TestCaseWithFactory):
 self.webservice.get(url)
 self.assertThat(recorder, HasQueryCount(Equals(15)))
 
-# XXX ines-almeida 2024-04-22: after adding the new API attribute
-# `build_metadata_url` to the snap builds, we actually perform an extra
-# query per build. We need to make a decision on whether we are OK with
-# this (and in such case, this test should be reworked or eventually
-# removed)
-#
-# def test_builds_query_count(self):
-# # The query count of Snap.builds is constant in the number of
-# # builds, even if they have store upload jobs.
-# self.pushConfig(
-# "snappy",
-# store_url="http://sca.example/;,
-# store_upload_url="http://updown.example/;,
-# )
-# with admin_logged_in():
-# snappyseries = self.factory.makeSnappySeries()
-# distroseries = self.factory.makeDistroSeries(
-# distribution=getUtility(IDistributionSet)["ubuntu"],
-# registrant=self.person,
-# )
-# processor = self.factory.makeProcessor(supports_virtualized=True)
-# distroarchseries = self.makeBuildab

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad-mojo-specs/+git/private:fetch-service-charmhub-configuration into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

Review: Approve

Looks good!

Can you update the commit message to start with the model in question?

I.e.
```
lp-fetch-service: Deploy fetch-service charm from Charmhub
```

Diff comments:

> diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
> index 8bd8dd3..67472ee 100644
> --- a/lp-fetch-service/bundle.yaml
> +++ b/lp-fetch-service/bundle.yaml
> @@ -16,9 +16,11 @@ applications:
>  public. Currently, this expects the charm and snap to be copied to 
> the
>  path where the bundle.yaml is rendered, in the mojo project folder 
> (eg.
>  .../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
> -charm: "./fetch-service_ubuntu-22.04-amd64.charm"
> -resources:
> -  snap: "./fetch-service.snap"
> +charm: ch:fetch-service
> +channel: edge
> +revision: 4
>  num_units: 1
>  expose: true
> +options:
> +channel: "latest/edge"

I'm guessing this is the snap channel? If so can you add a quick comment here 
saying so?

>  {%- endif %}


-- 
https://code.launchpad.net/~pelpsi/launchpad-mojo-specs/+git/private/+merge/464821
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master has been updated.

Commit message changed to:

Update `build_metadat_url` logic and tests and queries associated with it

This includes:
 - Update the query used to get the metadata file to be lighter
 - Remove test that no longer applies

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464751
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master.

Commit message:
Update `build_metadat_url` logic and tests and queries associated with it

This includes:
 - Update the query used to get the metadata file to be lighter
 - Remove test that no longer applies
 - Pre-load librarian file data when fetching all builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464751

All `build_metadata_url` tests re-ran.

I'm not 100% set if there is more to be done regarding keeping the query count 
stable regardless of builds, unless we store the link to the librarian file 
directly in the SnapBuild DB table. I'd love opinions on this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 1d08e03..9bacf76 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -11,6 +11,7 @@ token if and only if they are allowed general internet access.
 """
 
 __all__ = [
+"BUILD_METADATA_FILENAME_FORMAT",
 "BuilderProxyMixin",
 ]
 
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index c7418e6..0fc5d16 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -51,7 +51,7 @@ from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.person import Person
 from lp.services.config import config
-from lp.services.database.bulk import load_related
+from lp.services.database.bulk import load_referencing, load_related
 from lp.services.database.constants import DEFAULT
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
@@ -431,10 +431,10 @@ class SnapBuild(PackageBuildMixin, StormBase):
 metadata_filename = BUILD_METADATA_FILENAME_FORMAT.format(
 build_id=self.build_cookie
 )
-for url in self.getFileUrls():
-if url.endswith(metadata_filename):
-return url
-return None
+try:
+return self.lfaUrl(self.getFileByName(metadata_filename))
+except NotFoundError:
+return None
 
 @cachedproperty
 def eta(self):
@@ -663,6 +663,10 @@ class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
 )
 load_related(Job, sbjs, ["job_id"])
 
+# Prefetch snap files
+snap_files = load_referencing(SnapFile, builds, ["snapbuild_id"])
+load_related(LibraryFileAlias, snap_files, ["libraryfile_id"])
+
 def getByBuildFarmJobs(self, build_farm_jobs):
 """See `ISpecificBuildFarmJobSource`."""
 if len(build_farm_jobs) == 0:
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 9f671b8..39ba808 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -5797,65 +5797,3 @@ class TestSnapWebservice(TestCaseWithFactory):
 with StormStatementRecorder() as recorder:
 self.webservice.get(url)
 self.assertThat(recorder, HasQueryCount(Equals(15)))
-
-def test_builds_query_count(self):
-# The query count of Snap.builds is constant in the number of
-# builds, even if they have store upload jobs.
-self.pushConfig(
-"snappy",
-store_url="http://sca.example/;,
-store_upload_url="http://updown.example/;,
-)
-with admin_logged_in():
-snappyseries = self.factory.makeSnappySeries()
-distroseries = self.factory.makeDistroSeries(
-distribution=getUtility(IDistributionSet)["ubuntu"],
-registrant=self.person,
-)
-processor = self.factory.makeProcessor(supports_virtualized=True)
-distroarchseries = self.makeBuildableDistroArchSeries(
-distroseries=distroseries, processor=processor, owner=self.person
-)
-with person_logged_in(self.person):
-snap = self.factory.makeSnap(
-registrant=self.person,
-owner=self.person,
-distroseries=distroseries,
-processors=[processor],
-)
-snap.store_series = snappyseries
-snap.store_name = self.factory.getUniqueUnicode()
-snap.store_upload = True
-snap.store_secrets = {"root": Macaroon().serialize()}
-builds_url = "%s/builds" % api_url(snap)
-logout()
-
-def make_build():

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master

[Ines Almeida]

Diff comments:

> diff --git a/lib/lp/snappy/model/snapbuild.py 
> b/lib/lp/snappy/model/snapbuild.py
> index c7418e6..0fc5d16 100644
> --- a/lib/lp/snappy/model/snapbuild.py
> +++ b/lib/lp/snappy/model/snapbuild.py
> @@ -663,6 +663,10 @@ class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
>  )
>  load_related(Job, sbjs, ["job_id"])
>  
> +# Prefetch snap files

I'm not 100% sure about this. Regarding query counts, it didn't seem to have an 
effect. I added it as a commit that I can revert, but I'd like comments on this 
if there is any.

> +snap_files = load_referencing(SnapFile, builds, ["snapbuild_id"])
> +load_related(LibraryFileAlias, snap_files, ["libraryfile_id"])
> +
>  def getByBuildFarmJobs(self, build_farm_jobs):
>  """See `ISpecificBuildFarmJobSource`."""
>  if len(build_farm_jobs) == 0:


-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464751
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-update-build-metadata-url into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~jugmac00/launchpad:expose-build-metadata-via-api into launchpad:master

[Ines Almeida]

Changing this MP to Rejected in favor of this one: 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464697
-- 
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.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~jugmac00/launchpad:expose-build-metadata-via-api into launchpad:master

[Ines Almeida]

The proposal to merge ~jugmac00/launchpad:expose-build-metadata-via-api into 
launchpad:master has been updated.

Status: Needs review => Rejected

For more details, see:
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.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-expose-build-metadata-url-api into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-expose-build-metadata-url-api into 
launchpad:master with 
~ines-almeida/launchpad:fetch-service-buildd-manager-end-session as a 
prerequisite.

Commit message:
Expose the URL to download the metadata file of the fetch service via API

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464697
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-expose-build-metadata-url-api into 
launchpad:master.
diff --git a/lib/lp/snappy/interfaces/snapbuild.py b/lib/lp/snappy/interfaces/snapbuild.py
index 52019bf..3416200 100644
--- a/lib/lp/snappy/interfaces/snapbuild.py
+++ b/lib/lp/snappy/interfaces/snapbuild.py
@@ -340,6 +340,18 @@ class ISnapBuildView(IPackageBuildView, IPrivacy):
 _("A dict of data about store upload progress.")
 )
 
+build_metadata_url = exported(
+TextLine(
+title=_("URL of the build metadata file"),
+description=_(
+"URL of the metadata file generated by the fetch service, if "
+"it exists."
+),
+required=False,
+readonly=True,
+)
+)
+
 def getFiles():
 """Retrieve the build's `ISnapFile` records.
 
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index df2e2c2..c7418e6 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -34,6 +34,7 @@ from zope.interface.interfaces import ObjectEvent
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NotFoundError
+from lp.buildmaster.builderproxy import BUILD_METADATA_FILENAME_FORMAT
 from lp.buildmaster.enums import (
 BuildFarmJobType,
 BuildQueueStatus,
@@ -425,6 +426,16 @@ class SnapBuild(PackageBuildMixin, StormBase):
 def getFileUrls(self):
 return [self.lfaUrl(lfa) for _, lfa, _ in self.getFiles()]
 
+@property
+def build_metadata_url(self):
+metadata_filename = BUILD_METADATA_FILENAME_FORMAT.format(
+build_id=self.build_cookie
+)
+for url in self.getFileUrls():
+if url.endswith(metadata_filename):
+return url
+return None
+
 @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..c60b6ae 100644
--- a/lib/lp/snappy/tests/test_snapbuild.py
+++ b/lib/lp/snappy/tests/test_snapbuild.py
@@ -1009,6 +1009,57 @@ class TestSnapBuildWebservice(TestCaseWithFactory):
 for file_url in file_urls:
 self.assertCanOpenRedirectedUrl(browser, file_url)
 
+def test_build_metadata_url(self):
+# API clients can fetch the metadata from the build, generated by the
+# fetch service
+db_build = self.factory.makeSnapBuild(requester=self.person)
+metadata_filename = f"{db_build.build_cookie}_metadata.json"
+with person_logged_in(self.person):
+file_1 = self.factory.makeLibraryFileAlias(
+content="some_json",
+filename="test_file.json",
+)
+db_build.addFile(file_1)
+metadata_file = self.factory.makeLibraryFileAlias(
+content="some_json",
+filename=metadata_filename,
+)
+db_build.addFile(metadata_file)
+file_2 = self.factory.makeLibraryFileAlias(
+content="some_json",
+filename="another_test_file.tar",
+)
+db_build.addFile(file_2)
+
+build_url = api_url(db_build)
+logout()
+
+build = self.webservice.get(build_url).jsonBody()
+self.assertIsNotNone(build["build_metadata_url"])
+self.assertEndsWith(build["build_metadata_url"], metadata_filename)
+
+def test_build_metadata_url_no_metadata_file(self):
+# The attribute `build_metadata_url` returns None when metadata file
+# does not exist.
+db_build = self.factory.makeSnapBuild(requester=self.person)
+with person_logged_in(self.person):
+file_1 = self.factory.makeLibraryFileAlias(
+content="some_json",
+filename="test_file.json",
+)
+db_build.addFile(file_1)
+file_2 = self.factory.makeLibraryFileAlias(
+content="some_json",
+filename="another_test_file.tar",
+)
+db_build.addFile(file_2)
+
+build_url = api_url(db_build)
+

Re: [Launchpad-reviewers] [Merge] ~jugmac00/launchpad:expose-build-metadata-via-api into launchpad:master

[Ines Almeida]

Diff comments:

> diff --git a/lib/lp/snappy/interfaces/snapbuild.py 
> b/lib/lp/snappy/interfaces/snapbuild.py
> index 52019bf..fea9d5f 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")

As mentioned in the previous diff, this is being exported as a method, which 
makes it odd that it's named as an attribute. I think we either export it as 
`getBuildMetadataUrl()` (name of the method), or export it as an attribute. But 
calling `build.build_metadata_url()` when we use this in `lp-shell` is not 
coherent.

> +@export_read_operation()
> +@operation_for_version("devel")
> +def getBuildMetadataUrl():
> +"""URL of the metadata file generated by the fetch service
> +
> +:return: URL of the metadata if present."""
> +
>  
>  class ISnapBuildEdit(IBuildFarmJobEdit):
>  """`ISnapBuild` attributes that require launchpad.Edit."""


-- 
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.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master has been updated.

Commit message changed to:

Refactor buildd-manager: remove unneeded variables, rename and add comments

 - Removed the `/session` from the base URL for the fetch service control API 
endpoint, for consistency.
 - The `proxy_username` was confirmed to not be necessary for the fetch service.
 - Minor comment and variable refactoring

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464684
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.

Commit message:
Refactor buildd-manager: remove unneeded variables, rename and add comments 

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464684
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index a2b8f71..304c258 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -58,7 +58,7 @@ class BuilderProxyMixin:
 use_fetch_service: bool = False,
 ) -> Generator[None, Dict[str, str], None]:
 
-self.proxy_service = None
+self._proxy_service = None
 
 if not allow_internet:
 return
@@ -80,10 +80,10 @@ class BuilderProxyMixin:
 # non-production environments.
 return
 
-self.proxy_service = proxy_service(
+self._proxy_service = proxy_service(
 build_id=self.build.build_cookie, worker=self._worker
 )
-new_session = yield self.proxy_service.startSession()
+new_session = yield self._proxy_service.startSession()
 args["proxy_url"] = new_session["proxy_url"]
 args["revocation_endpoint"] = new_session["revocation_endpoint"]
 
@@ -181,7 +181,7 @@ class FetchService(IProxyService):
 """
 
 PROXY_URL = "http://{session_id}:{token}@{proxy_endpoint};
-START_SESSION_ENDPOINT = "{control_endpoint}"
+START_SESSION_ENDPOINT = "{control_endpoint}/session"
 TOKEN_REVOCATION = "{control_endpoint}/session/{session_id}/token"
 RETRIEVE_METADATA_ENDPOINT = "{control_endpoint}/session/{session_id}"
 END_SESSION_ENDPOINT = "{control_endpoint}/session/{session_id}"
@@ -217,18 +217,12 @@ class FetchService(IProxyService):
 
 See IProxyService.
 """
-timestamp = int(time.time())
-proxy_username = "{build_id}-{timestamp}".format(
-build_id=self.build_id, timestamp=timestamp
-)
-
 session_data = yield self.worker.process_pool.doWork(
 RequestFetchServiceSessionCommand,
 url=self.START_SESSION_ENDPOINT.format(
 control_endpoint=self.control_endpoint
 ),
 auth_header=self.auth_header,
-proxy_username=proxy_username,
 )
 
 self.session_id = session_data["id"]
diff --git a/lib/lp/buildmaster/downloader.py b/lib/lp/buildmaster/downloader.py
index 7cf1c9b..ba3eaf2 100644
--- a/lib/lp/buildmaster/downloader.py
+++ b/lib/lp/buildmaster/downloader.py
@@ -49,7 +49,6 @@ class RequestFetchServiceSessionCommand(amp.Command):
 arguments = [
 (b"url", amp.Unicode()),
 (b"auth_header", amp.String()),
-(b"proxy_username", amp.Unicode()),
 ]
 response = [
 (b"id", amp.Unicode()),
@@ -119,9 +118,7 @@ class RequestProcess(AMPChild):
 return response.json()
 
 @RequestFetchServiceSessionCommand.responder
-def requestFetchServiceSessionCommand(
-self, url, auth_header, proxy_username
-):
+def requestFetchServiceSessionCommand(self, url, auth_header):
 with Session() as session:
 session.trust_env = False
 # XXX pelpsi: from ST108 and from what Claudio
@@ -134,7 +131,7 @@ class RequestProcess(AMPChild):
 response = session.post(
 url,
 headers={"Authorization": auth_header},
-json={"username": proxy_username, "policy": "permissive"},
+json={"policy": "permissive"},
 )
 response.raise_for_status()
 return response.json()
diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
index 1ba8b7b..dfcc86b 100644
--- a/lib/lp/buildmaster/interactor.py
+++ b/lib/lp/buildmaster/interactor.py
@@ -3,6 +3,7 @@
 
 __all__ = [
 "BuilderInteractor",
+"BuilderWorker",
 "extract_vitals_from_db",
 ]
 
diff --git a/lib/lp/buildmaster/tests/fetchservice.py b/lib/lp/buildmaster/tests/fetchservice.py
index 51f73e3..16fabcb 100644
--- a/lib/lp/buildmaster/tests/fetchservice.py
+++ b/lib/lp/buildmaster/tests/fetchservice.py
@@ -17,16 +17,18 @@ from twisted.web import resource, server
 from lp.services.config import config
 
 
-class FetchServiceAuthAPITokensResource(resource.Resource):
-"""A test session resource for 

Re: [Launchpad-reviewers] [Merge] ~jugmac00/launchpad:expose-build-metadata-via-api into launchpad:master

[Ines Almeida]

I know this isn't tested, but just a initial review!

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 think this will still be exported a method, and if so we usually go for camel 
case. Maybe we want to export it as a attribute instead?

> +@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."""
> +
>  
>  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:

Maybe it will be good return `None` if `use_fetch_service` is False?

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

If we do go for `{build_cookie}_metadata.json` we can actually check the full 
name of the file.

--
We should add a comment here referencing the part in the code where 
`metadata.json` is defined (which will only be merged in my open MP). If this 
one is merged first, I'll add it on my side.

> +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):

I think this test should have a couple more files created for the snap to make 
the test more meaningful, and maybe a negative test that returns `None` when 
the file isn't there (which will be the case for most snaps).

> +# 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(
> +content="some_json",
> +filename="xxx-metadata.json",

In my WIP I had `xxx_metadata.json`, (with an underscore), I think it's best to 
keep it as close as possible, but we can also change from my side.

> +)
> +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.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-buildd-manager-end-session into launchpad:master

[Ines Almeida]

Diff comments:

> diff --git a/lib/lp/buildmaster/builderproxy.py 
> b/lib/lp/buildmaster/builderproxy.py
> index a2b8f71..7b1c919 100644
> --- a/lib/lp/buildmaster/builderproxy.py
> +++ b/lib/lp/buildmaster/builderproxy.py
> @@ -87,6 +98,41 @@ class BuilderProxyMixin:
>  args["proxy_url"] = new_session["proxy_url"]
>  args["revocation_endpoint"] = new_session["revocation_endpoint"]
>  
> +def endProxySession(self, upload_path):
> +if not self.use_fetch_service:
> +# No cleanup needed for the buidler proxy
> +return
> +
> +if self.proxy_service is None:
> +# A session was never started
> +return
> +
> +if not isinstance(self.proxy_service, FetchService):
> +# There shouldn't be any instance where this error is called,
> +# yet it's best to be sure of the `self.proxy_service` type.
> +raise ProxyServiceException(
> +"Expected a `FetchService` session, but got a "
> +f"`{self.proxy_service.__class__.__name__}` session instead."
> +)
> +
> +# Fetch session metadata from the proxy service and store in the same
> +# path as other build files. If storing or retrieving the file fails,
> +# they will raise an exception, meaning we don't close the session.
> +# Sessions will be closed automatically within the Fetch Service 
> after
> +# a certain amount of time configured by its charm (default 6 hours).
> +metadata = yield self.proxy_service.retrieveMetadataFromSession()
> +file_path = os.path.join(
> +upload_path, f"{self.build.build_cookie}_metadata.json"

> What is your reasoning to use the naming you chose?

This is certainly not set in stone and something I still wanted to check. I had 
the same question about whether to have a fixed name, and thought maybe it 
would be best to find some variable that would uniquely identify it.

The build_cookie is a unique identifier for the build job (str).

Let's have a chat about this, because I'd also like to define what the name of 
the file should be.

> +)
> +yield self._worker.process_pool.doWork(
> +SaveAsFileCommand,
> +content=metadata,
> +path_to_write=file_path,
> +)
> +
> +# Once metadata is saved, we close the session (which deletes its 
> data)
> +yield self.proxy_service.endSession()
> +
>  
>  class IProxyService:
>  """Interface for Proxy Services - either FetchService or BuilderProxy."""
> diff --git a/lib/lp/buildmaster/downloader.py 
> b/lib/lp/buildmaster/downloader.py
> index 7cf1c9b..55a13e7 100644
> --- a/lib/lp/buildmaster/downloader.py
> +++ b/lib/lp/buildmaster/downloader.py
> @@ -38,6 +40,17 @@ class DownloadCommand(amp.Command):
>  }
>  
>  
> +class SaveAsFileCommand(amp.Command):

Indeed, this one was mostly copied from the DownloadCommand, and I'm not that 
knowledgeable about this part of the code either.

The main difference is that the download command streams the data from a file 
to write to another, and in this one we already start with the data we want to 
save so we simply write. It didn't feel like I could just use the same command, 
but we can certainly see if we can with a few adjustments.

My plan is to get it working as is, test it, and then think about cleaning up 
and optimizing.

> +arguments = [
> +(b"content", amp.Unicode()),
> +(b"path_to_write", amp.Unicode()),
> +]
> +response: List[Tuple[bytes, amp.Argument]] = []
> +errors = {
> +RequestException: b"REQUEST_ERROR",
> +}
> +
> +
>  class RequestFetchServiceSessionCommand(amp.Command):
>  """Fetch service API Command subclass
>  


-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464312
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-buildd-manager-end-session into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.

Commit message:
Update fetch service revoke token authentication

We now use the proxy token as authentication to the fetch service control API 
to revoke itself.
This is the only control endpoint that can be authenticated with the proxy 
token, as all it can do it revoke itself.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/464529
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.
diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py
index ab65f5f..8a5d766 100644
--- a/lpbuildd/tests/test_util.py
+++ b/lpbuildd/tests/test_util.py
@@ -92,7 +92,8 @@ class TestRevokeToken(TestCase):
 def test_revoke_fetch_service_token(self):
 """Proxy token revocation for the fetch service"""
 
-proxy_url = "http://session_id:token@proxy.fetch-service.example;
+token = "token"
+proxy_url = f"http://session_id:{token}@proxy.fetch-service.example;
 revocation_endpoint = (
 "http://control.fetch-service.example/session_id/token;
 )
@@ -111,3 +112,4 @@ class TestRevokeToken(TestCase):
 "http://control.fetch-service.example/session_id/token;,
 request.url,
 )
+self.assertEqual(f"Basic {token}", request.headers["Authorization"])
diff --git a/lpbuildd/util.py b/lpbuildd/util.py
index 664f92b..b09b33f 100644
--- a/lpbuildd/util.py
+++ b/lpbuildd/util.py
@@ -1,6 +1,7 @@
 # Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import base64
 import os
 import subprocess
 import sys
@@ -81,21 +82,25 @@ def revoke_proxy_token(
 authentication to revoke its token.
 
 If using the fetch service:
-The call to revoke a token does not require authentication.
+The proxy_url for the Fetch Service has the following format:
+http://{session_id}:{token}@{host}:{port}
 
-XXX ines-almeida 2024-04-15: this might change depending on
-conversations about fetch service authentication. We might decide to
-instead use the token itself as the authentication.
+We use the token from the proxy_url for authentication to revoke
+elself.
 
 :raises RevokeProxyTokenError: if attempting to revoke the token failed.
 """
 url = urlparse(proxy_url)
 
-auth = None
 if not use_fetch_service:
-auth = (url.username, url.password)
+auth_string = f"{url.username}:{url.password}"
+token = base64.b64encode(auth_string.encode()).decode()
+else:
+token = url.password
+
+headers = {"Authorization": f"Basic {token}"}
 
 try:
-requests.delete(revocation_endpoint, auth=auth, timeout=15)
+requests.delete(revocation_endpoint, headers=headers, timeout=15)
 except requests.RequestException as e:
 raise RevokeProxyTokenError(url.username, e)
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master

[Ines Almeida]

Fixed all mypy issues (interestingly, some of them were issues that already 
existed in the code previously, but weren't an issue before), and added 
comments where it was relevant.

I will refrain from adding unit tests in this MP since the functionality seems 
to be highly tested between the multiple tests that contain `extraBuildArgs` 
and all the tests within `lp.snappy.tests.test_snapbuildbehaviour`.
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464303
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.

Commit message:
Refactor buildd-manager builder proxy logic

This separates the logic for making calls to the builder proxy and the fetch 
service out of the BuilderProxyMixin and into their own handlers.
This will make it easier to later add the logic to end sessions or, retreieve 
metadata.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464303

All tests in `lp.snappy.tests` have be run and passed successfully.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 3375042..e4da105 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -49,79 +49,159 @@ class BuilderProxyMixin:
 self,
 args: BuildArgs,
 allow_internet: bool = True,
-fetch_service: bool = False,
+use_fetch_service: bool = False,
 ) -> Generator[None, Dict[str, str], None]:
 if not allow_internet:
 return
-if not fetch_service and _get_proxy_config("builder_proxy_host"):
-token = yield self._requestProxyToken()
-args["proxy_url"] = (
-"http://{username}:{password}@{host}:{port}".format(
-username=token["username"],
-password=token["secret"],
-host=_get_proxy_config("builder_proxy_host"),
-port=_get_proxy_config("builder_proxy_port"),
-)
-)
-args["revocation_endpoint"] = "{endpoint}/{token}".format(
-endpoint=_get_proxy_config("builder_proxy_auth_api_endpoint"),
-token=token["username"],
-)
-elif fetch_service and _get_proxy_config("fetch_service_host"):
-session = yield self._requestFetchServiceSession()
-args["proxy_url"] = (
-"http://{session_id}:{token}@{host}:{port}".format(
-session_id=session["id"],
-token=session["token"],
-host=_get_proxy_config("fetch_service_host"),
-port=_get_proxy_config("fetch_service_port"),
-)
-)
-args["revocation_endpoint"] = "{endpoint}/{session_id}".format(
-endpoint=_get_proxy_config("fetch_service_control_endpoint"),
-session_id=session["id"],
-)
+if not use_fetch_service and _get_proxy_config("builder_proxy_host"):
+proxy_service = BuilderProxy
+elif use_fetch_service and _get_proxy_config("fetch_service_host"):
+proxy_service = FetchService
+else:
+return
+
+self.proxy_service = proxy_service(
+build_id=self.build.build_cookie, worker=self._worker
+)
+new_session = yield self.proxy_service.startSession()
+args["proxy_url"] = new_session["proxy_url"]
+args["revocation_endpoint"] = new_session["revocation_endpoint"]
+
+
+class BuilderProxy:
+
+def __init__(self, build_id, worker):
+self.control_endpoint = _get_value_from_config(
+"builder_proxy_auth_api_endpoint"
+)
+self.proxy_endpoint = "{host}:{port}".format(
+host=_get_proxy_config("builder_proxy_host"),
+port=_get_proxy_config("builder_proxy_port"),
+)
+self.auth_header = self._getAuthHeader()
+
+self.build_id = build_id
+self.worker = worker
+
+@staticmethod
+def _getAuthHeader():
+"""Helper method to generate authentication needed to call the
+builder proxy authentication endpoint."""
 
-@defer.inlineCallbacks
-def _requestProxyToken(self):
 admin_username = _get_value_from_config(
 "builder_proxy_auth_api_admin_username"
 )
-secret = _get_value_from_config("builder_proxy_auth_api_admin_secret")
-url = _get_value_from_config("builder_proxy_auth_api_endpoint")
+admin_secret = _get_value_from_config(
+"builder_proxy_auth_api_admin_secret"
+)
+auth_string = f"{admin_username}:{admin_secret}".strip()
+return b"Basic " + base64.b64encode(auth_string.encode("AS

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master

[Ines Almeida]

The code itself should be tested from the existing unit tests, since the 
functionality was maintained. But I still need to have a look at adding new 
tests for the new classes.
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464303
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

 self.buildmanager.revokeProxyToken()
> +
> +self.assertEqual(1, len(responses.calls))
> +request = responses.calls[0].request
> +self.assertEqual(
> +f"http://fetch-service.example/{session_id}/token;, request.url
> +)

There are a lot of places within this file and the rest of the repo that use 
the `.example`, so I think I might be more consistent to keep it IMO. I updated 
the URLs though.

I called it `control.fetch-service` instead of `orchestration` because I think 
that's been the most common name from the specs.

> diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py
> index 005d42b..15c861c 100644
> --- a/lpbuildd/tests/test_util.py
> +++ b/lpbuildd/tests/test_util.py
> @@ -59,3 +67,43 @@ class TestSetPersonality(TestCase):
>  ["linux64", "sbuild"],
>  set_personality(["sbuild"], "amd64", series="trusty"),
>  )
> +
> +
> +class TestRevokeToken(TestCase):
> +@responses.activate
> +def test_revoke_proxy_token(self):
> +"""Proxy token revocation uses the right authentication"""
> +
> +proxy_url = "http://username:password@host:port;
> +revocation_endpoint = "http://builder.api.endpoint.token;

I updated the URLs to use the exact same ones as the previous tests. Note that 
this particular one is for the builder proxy (not the fetch service)

> +token = base64.b64encode(b"username:password").decode()
> +
> +responses.add(responses.DELETE, revocation_endpoint)
> +
> +revoke_proxy_token(proxy_url, revocation_endpoint)
> +self.assertEqual(1, len(responses.calls))
> +request = responses.calls[0].request
> +self.assertEqual("http://builder.api.endpoint.token/;, request.url)
> +self.assertEqual(f"Basic {token}", request.headers["Authorization"])
> +
> +@responses.activate
> +def test_revoke_fetch_service_token(self):
> +"""Proxy token revocation for the fetch service"""
> +
> +token = "test-token"
> +proxy_url = f"http://session_id:{token}@host:port;
> +revocation_endpoint = "http://fetch-service.example/session_id/token;

As mentioned, the rest of this repo has a lot of instances of `.example`... I 
think renaming these would only make sense by renaming all others, and I think 
that should go into a separate issue. I updated this to be the same as in the 
tests above though

> +
> +responses.add(responses.DELETE, revocation_endpoint)
> +
> +revoke_proxy_token(
> +proxy_url,
> +revocation_endpoint,
> +use_fetch_service=True,
> +)
> +
> +self.assertEqual(1, len(responses.calls))
> +request = responses.calls[0].request
> +self.assertEqual(
> +"http://fetch-service.example/session_id/token;, request.url
> +)
> diff --git a/lpbuildd/util.py b/lpbuildd/util.py
> index eb35f01..0171e1f 100644
> --- a/lpbuildd/util.py
> +++ b/lpbuildd/util.py
> @@ -68,15 +68,34 @@ class RevokeProxyTokenError(Exception):
>  return f"Unable to revoke token for {self.username}: 
> {self.exception}"
>  
>  
> -def revoke_proxy_token(proxy_url, revocation_endpoint):
> +def revoke_proxy_token(
> +proxy_url, revocation_endpoint, use_fetch_service=False
> +):
>  """Revoke builder proxy token.
>  
> +If not using the fetch service:
> +The proxy_url for the current Builder Proxy has the following format:
> +http://{username}:{password}@{host}:{port}
> +
> +We use the username-password combo from the proxy_url for
> +authentication to revoke its token.
> +
> +If using thefetch service:

Updated

> +The call to revoke a token does not require authentication.
> +
> +TODO this might change depending on conversations about fetch service

IDEs should highlight the `TODO` comments as much as the `XXX` ones since it's 
a generally used format as well (https://peps.python.org/pep-0350/)  - mine 
does at least.

Regardless, IMO this should only get merged after that gets defined - this TODO 
was a note to the reviewer and to myself. I updated it since it's not a lot of 
work

> +authentication. We might decide to instead use the token itself as 
> the
> +authentication.
> +
>  :raises RevokeProxyTokenError: if attempting to revoke the token failed.
>  """
>  url = urlparse(proxy_url)
> +
> +auth = None
> +if not use_fetch_service:
> +auth = (url.username, url.password)
> +
>  try:
> -requests.delete(
> -revocation_endpoint, auth=(url.username, url.password), 
> timeout=15
> -)
> +requests.delete(revocation_endpoint, auth=auth, timeout=15)
>  except requests.RequestException as e:
>  raise RevokeProxyTokenError(url.username, e)


-- 
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

This MP has been updated after some conversations. The topic of how to 
authenticate the call to revoke the token is still on not decided on, so that 
part might change.
-- 
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Status: Work in progress => Rejected

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/463119
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into 
launchpad:master has been updated.

Status: Work in progress => Rejected

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/463145
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:change-appserver-log-rotate into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:change-appserver-log-rotate into launchpad:master.

Commit message:
charm: update appserver logrotate from daily to hourly

Currently log files for this service can get quite heavy (in the realm of 10 
Gb) which makes it impossible to live view the latest logs of a unit. Taking 
into account that we run 4 appserver units, debuging an issue can lead to 
several minutes to download the daily log files, and the launchpad-bastion disk 
to get unnecessarily full.
In this change, we are rotating the logs hourly, while keeping the same amount 
of days worth of logs on disk. This will lead to lighter log files, and less 
space for logs used.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464017

This is currently running in qastaging (changed manually) for testing
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:change-appserver-log-rotate into launchpad:master.
diff --git a/charm/launchpad-appserver/templates/logrotate.conf.j2 b/charm/launchpad-appserver/templates/logrotate.conf.j2
index 494f246..2714525 100644
--- a/charm/launchpad-appserver/templates/logrotate.conf.j2
+++ b/charm/launchpad-appserver/templates/logrotate.conf.j2
@@ -1,8 +1,9 @@
 {{ logs_dir }}/launchpad.log
 {
-rotate 21
-daily
+rotate 504
+hourly
 dateext
+dateformat -%Y%m%d-%H
 delaycompress
 compress
 notifempty
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:fetch-service-fix-typo into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:fetch-service-fix-typo into 
~launchpad/launchpad-mojo-specs/+git/private:master has been updated.

Status: Needs review => Approved

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/463831
-- 
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:fetch-service-fix-typo into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:fetch-service-fix-typo into 
~launchpad/launchpad-mojo-specs/+git/private:master.

Commit message:
lp-fetch-service: fix small typo in mojo spec

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/463831
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-mojo-specs/+git/private:fetch-service-fix-typo into 
~launchpad/launchpad-mojo-specs/+git/private:master.
diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
index 34bcc52..8bd8dd3 100644
--- a/lp-fetch-service/bundle.yaml
+++ b/lp-fetch-service/bundle.yaml
@@ -20,5 +20,5 @@ applications:
 resources:
   snap: "./fetch-service.snap"
 num_units: 1
-exposed: true
+expose: true
 {%- endif %}
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into 
launchpad:master has been updated.

Status: Needs review => Work in progress

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/463145
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Status: Needs review => Work in progress

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/463119
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ruinedyourlife/lp-archive:feat-landing-page into lp-archive:main

[Ines Almeida]

Review: Approve


-- 
https://code.launchpad.net/~ruinedyourlife/lp-archive/+git/lp-archive/+merge/463798
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ruinedyourlife/lp-archive:feat-landing-page into lp-archive:main.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~pelpsi/launchpad:snap-component-integration into launchpad:master

[Ines Almeida]

You have been requested to review the proposed merge of 
~pelpsi/launchpad:snap-component-integration into launchpad:master.

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/461063



-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pelpsi/launchpad:snap-component-integration into launchpad:master.
diff --git a/lib/lp/snappy/interfaces/snapbuildjob.py b/lib/lp/snappy/interfaces/snapbuildjob.py
index 12e4b8b..90a8d37 100644
--- a/lib/lp/snappy/interfaces/snapbuildjob.py
+++ b/lib/lp/snappy/interfaces/snapbuildjob.py
@@ -13,7 +13,7 @@ __all__ = [
 from lazr.restful.fields import Reference
 from zope.interface import Attribute, Interface
 from zope.interface.interfaces import IObjectEvent
-from zope.schema import Int, TextLine
+from zope.schema import Dict, Int, TextLine
 
 from lp import _
 from lp.services.job.interfaces.job import IJob, IJobSource, IRunnableJob
@@ -68,6 +68,17 @@ class ISnapStoreUploadJob(IRunnableJob):
 readonly=True,
 )
 
+components_ids = Dict(
+title=_(
+"The IDs returned by the store when uploading snap components."
+"The key is the component name and the value is the related id."
+),
+key_type=TextLine(),
+value_type=TextLine(),
+required=False,
+readonly=True,
+)
+
 status_url = TextLine(
 title=_("The URL on the store to get the status of this build"),
 required=False,
diff --git a/lib/lp/snappy/model/snapbuildjob.py b/lib/lp/snappy/model/snapbuildjob.py
index 539fb1c..80260c1 100644
--- a/lib/lp/snappy/model/snapbuildjob.py
+++ b/lib/lp/snappy/model/snapbuildjob.py
@@ -29,6 +29,12 @@ from lp.services.database.stormbase import StormBase
 from lp.services.job.model.job import EnumeratedSubclass, Job
 from lp.services.job.runner import BaseRunnableJob
 from lp.services.propertycache import get_property_cache
+from lp.snappy.interfaces.snap import (
+CannotFetchSnapcraftYaml,
+CannotParseSnapcraftYaml,
+ISnapSet,
+MissingSnapcraftYaml,
+)
 from lp.snappy.interfaces.snapbuildjob import (
 ISnapBuildJob,
 ISnapBuildStoreUploadStatusChangedEvent,
@@ -244,6 +250,18 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
 self.snapbuild.store_upload_metadata["upload_id"] = upload_id
 
 @property
+def components_ids(self):
+"""See `ISnapStoreUploadJob`."""
+return self.store_metadata.get("components_ids")
+
+@components_ids.setter
+def components_ids(self, components_ids):
+"""See `ISnapStoreUploadJob`."""
+if self.snapbuild.store_upload_metadata is None:
+self.snapbuild.store_upload_metadata = {}
+self.snapbuild.store_upload_metadata["components_ids"] = components_ids
+
+@property
 def status_url(self):
 """See `ISnapStoreUploadJob`."""
 return self.store_metadata.get("status_url")
@@ -333,6 +351,24 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
 pass
 return timedelta(minutes=1)
 
+@staticmethod
+def _extract_component_name(filename, snapcraft_components):
+"""Extract component name from filename
+
+Use the snapcraft_components keys to extract the component name
+from the built component filename. Each key correspond to
+ while the built filename has the following pattern:
++_.comp.
+"""
+for component in snapcraft_components:
+# Surround component name with `+` and `_` to avoid
+# superset of the same component name.
+# ie. testcomp -> testcomponent
+built_component = "+%s_" % component
+if built_component in filename:
+return component
+return filename
+
 def run(self):
 """See `IRunnableJob`."""
 client = getUtility(ISnapStoreClient)
@@ -350,16 +386,54 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
 # Nothing to do.
 self.error_message = None
 return
+
+components_ids = {}
+components = []
+try:
+# Get components from snapcraft.yaml
+snapcraft_yaml = getUtility(ISnapSet).getSnapcraftYaml(
+self.snapbuild.snap
+)
+if snapcraft_yaml["components"]:
+for component in snapcraft_yaml["components"]:
+components_ids[component] = None
+# Get built components from built files
+for _, lfa, _ in self.snapbuild.getFiles():
+if lfa.filename.endswith(".comp"):
+components.append(lfa)
+except (
+MissingSnapcraftYaml,
+CannotFetchSnapcraftYaml,
+CannotParseSnapcraftYaml,
+ 

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master with 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service as a 
prerequisite.

Commit message:
End proxy session and gather data when using the fetch service

We send an API request to the fetch service to end the proxy session, save the 
data from the response locally, and upload it as a build file

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/463119
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master.
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py
index fdaf5ca..a5d9232 100644
--- a/lpbuildd/proxy.py
+++ b/lpbuildd/proxy.py
@@ -12,7 +12,11 @@ from twisted.python.compat import intToBytes
 from twisted.web import http, proxy
 from zope.interface import implementer
 
-from lpbuildd.util import RevokeProxyTokenError, revoke_proxy_token
+from lpbuildd.util import (
+RevokeProxyTokenError,
+end_fetch_service_session,
+revoke_proxy_token,
+)
 
 
 class BuilderProxyClient(proxy.ProxyClient):
@@ -262,3 +266,20 @@ class BuildManagerProxyMixin:
 )
 except RevokeProxyTokenError as e:
 self._builder.log(f"{e}\n")
+
+def endProxySession(self, output_file):
+""" When using the fetch service, we want to end the session at the end
+of a build and save the metadata from the session.
+
+This file can be uploaded to the librarian as any other build file.
+"""
+if not self._use_fetch_service:
+return
+
+response = end_fetch_service_session(
+self.proxy_url,
+self.end_session_endpoint,
+)
+
+with self.backend.open(output_file, "wb") as f:
+f.write(response.content)
diff --git a/lpbuildd/snap.py b/lpbuildd/snap.py
index c5b3205..3033ced 100644
--- a/lpbuildd/snap.py
+++ b/lpbuildd/snap.py
@@ -41,6 +41,7 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 self.use_fetch_service = extra_args.get("use_fetch_service")
 self.proxy_url = extra_args.get("proxy_url")
 self.revocation_endpoint = extra_args.get("revocation_endpoint")
+self.end_session_endpoint = extra_args.get("end_session_endpoint")
 self.build_source_tarball = extra_args.get(
 "build_source_tarball", False
 )
@@ -153,3 +154,9 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 )
 if self.backend.path_exists(source_tarball_path):
 self.addWaitingFileFromBackend(source_tarball_path)
+
+if self.use_fetch_service:
+session_file = os.path.join(output_path, "session-data.json")
+self.endProxySession(output_file=session_file)
+if self.backend.path_exists(session_file):
+self.addWaitingFileFromBackend(session_file)
diff --git a/lpbuildd/tests/test_snap.py b/lpbuildd/tests/test_snap.py
index a31934a..dfd4b1c 100644
--- a/lpbuildd/tests/test_snap.py
+++ b/lpbuildd/tests/test_snap.py
@@ -3,6 +3,7 @@
 
 import base64
 import os
+from unittest import mock
 
 import responses
 from fixtures import EnvironmentVariable, TempDir
@@ -18,6 +19,7 @@ from lpbuildd.tests.fakebuilder import FakeBuilder
 from lpbuildd.tests.matchers import HasWaitingFiles
 
 
+
 class MockBuildManager(SnapBuildManager):
 def __init__(self, *args, **kwargs):
 super().__init__(*args, **kwargs)
@@ -906,3 +908,36 @@ class TestSnapBuildManagerIteration(TestCase):
 f"http://fetch-service.example/{session_id}/token;, request.url
 )
 
+@responses.activate
+def test_endProxySession(self):
+session_id = "123"
+token = "test_token"
+metadata = {"test": "data"}
+output_file = "output_file.json"
+
+responses.add(
+"DELETE",
+f"http://fetch-service.example/{session_id};,
+json=metadata,
+)
+
+self.buildmanager.backend = mock.MagicMock()
+self.buildmanager.use_fetch_service = True
+self.buildmanager.end_session_endpoint = (
+f"http://fetch-service.example/{session_id};
+)
+self.buildmanager.proxy_url = f"http://session_id:{token}@proxy.example;
+
+self.buildmanager.endProxySession(output_file)
+
+self.assertEqual(1, len(responses.calls))
+request = responses.calls[0].request
+self.assertEqual(f"Basic {token}", request.headers["Authorization"

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

I would still like to spend some more time going over where is the best place 
to call the method to save the fetch service data, but I think this is in a 
good enough state for a review.
-- 
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/463119
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into 
launchpad:master.

Commit message:
Send end_session_endpoint to build manager when using fetch service

The fetch service will require 2 endpoints: one for revoking the token (as the 
old builder proxy did) and one for ending the session and gathering the data - 
that we are adding in this MP

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/463145
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-add-get-data-endpoint into 
launchpad:master.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 3375042..9309246 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -77,10 +77,10 @@ class BuilderProxyMixin:
 port=_get_proxy_config("fetch_service_port"),
 )
 )
-args["revocation_endpoint"] = "{endpoint}/{session_id}".format(
-endpoint=_get_proxy_config("fetch_service_control_endpoint"),
-session_id=session["id"],
-)
+endpoint = _get_proxy_config("fetch_service_control_endpoint")
+session_id = session["id"]
+args["revocation_endpoint"] = f"{endpoint}/{session_id}/token"
+args["end_session_endpoint"] = f"{endpoint}/{session_id}"
 
 @defer.inlineCallbacks
 def _requestProxyToken(self):
diff --git a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
index 6a808b3..2afb876 100644
--- a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
@@ -128,6 +128,10 @@ class BuildArgs(TypedDict, total=False):
 # The URL for revoking proxy authorization tokens [charm, ci, oci,
 # snap].
 revocation_endpoint: str
+# The URL for ending a fetch service session (separate from the
+# 'revocation_endpoint, as is certain cases want to revoke the token before
+# closing the session)
+end_session_endpoint: str
 # If True, scan job output for malware [ci].
 scan_malware: bool
 # A dictionary of secrets to pass to the CI build runner [ci].
diff --git a/lib/lp/buildmaster/tests/builderproxy.py b/lib/lp/buildmaster/tests/builderproxy.py
index fece785..1f47c71 100644
--- a/lib/lp/buildmaster/tests/builderproxy.py
+++ b/lib/lp/buildmaster/tests/builderproxy.py
@@ -10,7 +10,12 @@ from textwrap import dedent
 from urllib.parse import urlsplit
 
 import fixtures
-from testtools.matchers import Equals, HasLength, MatchesStructure
+from testtools.matchers import (
+Equals,
+HasLength,
+MatchesRegex,
+MatchesStructure,
+)
 from twisted.internet import defer, endpoints, reactor
 from twisted.python.compat import nativeString
 from twisted.web import resource, server
@@ -120,3 +125,26 @@ class RevocationEndpointMatcher(Equals):
 int(now),
 )
 )
+
+
+class FetchServiceRevocationEndpointMatcher(MatchesRegex):
+"""Check that a string is a valid endpoint for proxy token revocation.
+
+Expected type: {endpoint}/{session_id:int}/token
+"""
+
+def __init__(self):
+endpoint = config.builddmaster.fetch_service_control_endpoint
+super().__init__(r"%s/\d/token" % (endpoint))
+
+
+class FetchServiceEndSessionEndpointMatcher(MatchesRegex):
+"""Check that a string is a valid endpoint for the fetch service session
+closing.
+
+Expected type: {endpoint}/{session_id:int}
+"""
+
+def __init__(self):
+endpoint = config.builddmaster.fetch_service_control_endpoint
+super().__init__(r"%s/\d" % (endpoint))
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 2ccc18d..efab726 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -47,6 +47,8 @@ from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
 )
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.buildmaster.tests.builderproxy import (
+FetchServiceEndSessionEndpointMatcher,
+FetchServiceRevocationEndpointMatcher,
 InProcessProxyAuthAPIFixture,
 ProxyURLMatcher,
 RevocationEndpointMatcher,
@@ -287,7 +289,8 @@ class TestAsyncSnapBuildBehaviourFetchService(
 configuration.
 
 If `fetch_service_host` is not provided the function will return
-without populating `proxy_url` and `revocation_endpoint`.
+without populating `proxy_url`,

Re: [Launchpad-reviewers] [Merge] ~ruinedyourlife/launchpad:feat-archive-metadata-overrides into launchpad:master

[Ines Almeida]

LGTM! Just a very small suggestion.

I was wondering if there are any tests to update or add, but maybe not since 
this new model attribute isn't really used yet - we'll do it when adding 
APIs/UI.

Note that this cannot be merged until the DB change is deployed to production. 
I'm happy to approve this one, but I won't for now because of that :)

Diff comments:

> diff --git a/lib/lp/soyuz/interfaces/archive.py 
> b/lib/lp/soyuz/interfaces/archive.py
> index e1521ce..cc48143 100644
> --- a/lib/lp/soyuz/interfaces/archive.py
> +++ b/lib/lp/soyuz/interfaces/archive.py
> @@ -1021,6 +1021,8 @@ class IArchiveView(IHasBuildRecords):
>  "pending publications."
>  )
>  
> +metada_overrides = Attribute("A dict of data about the archive.")

"A dict of data" sounds a little bit generic IMO, how about "An object 
containing metadata overrides for this archive." or something along those lines?

> +
>  processors = exported(
>  CollectionField(
>  title=_("Processors"),


-- 
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/463030
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ruinedyourlife/launchpad:feat-archive-metadata-overrides into launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Description changed to:

This handles calling the revocation endpoint against the fetch service, but not 
yet closing the session (that will be handled in another MP: 
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/463119)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Description changed to:

This handles calling the revocation endpoint against the fetch service, but not 
yet closing the session (that will be handled in another MP).

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Description changed to:



For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Commit message changed to:

Enable revoking a token for the fetch service

Handle calling the revocation_endpoint sent by Launchpad, both in the cases 
where we want to revoke the token after the snap pull phase, and at the end of 
the build

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.

Commit message:
WIP: Update end-of-session code for snaps that use the fetch service

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875

This is WIP code with some TODOs and open questions to answer.
The goal is to get early feedback and direction.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py
index ea33dda..b56d890 100644
--- a/lpbuildd/proxy.py
+++ b/lpbuildd/proxy.py
@@ -214,8 +214,20 @@ class BuilderProxyFactory(http.HTTPFactory):
 
 
 class BuildManagerProxyMixin:
+
+@property
+def _use_fetch_service(self):
+return (
+hasattr(self, 'use_fetch_service')
+and getattr(self, "use_fetch_service")
+)
+
 def startProxy(self):
-"""Start the local builder proxy, if necessary."""
+"""Start the local builder proxy, if necessary.
+
+This starts an internal proxy that stands before the Builder
+Proxy/Fetch Service. It is important for certain build types.
+"""
 if not self.proxy_url:
 return []
 proxy_port = self._builder._config.get("builder", "proxyport")
@@ -243,6 +255,10 @@ class BuildManagerProxyMixin:
 return
 self._builder.log("Revoking proxy token...\n")
 try:
-revoke_proxy_token(self.proxy_url, self.revocation_endpoint)
+revoke_proxy_token(
+self.proxy_url,
+self.revocation_endpoint,
+self._use_fetch_service,
+)
 except RevokeProxyTokenError as e:
 self._builder.log(f"{e}\n")
diff --git a/lpbuildd/snap.py b/lpbuildd/snap.py
index 02390d4..c5b3205 100644
--- a/lpbuildd/snap.py
+++ b/lpbuildd/snap.py
@@ -38,6 +38,7 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 self.branch = extra_args.get("branch")
 self.git_repository = extra_args.get("git_repository")
 self.git_path = extra_args.get("git_path")
+self.use_fetch_service = extra_args.get("use_fetch_service")
 self.proxy_url = extra_args.get("proxy_url")
 self.revocation_endpoint = extra_args.get("revocation_endpoint")
 self.build_source_tarball = extra_args.get(
@@ -100,6 +101,8 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 if self.target_architectures:
 for arch in self.target_architectures:
 args.extend(["--target-arch", arch])
+if self.use_fetch_service:
+args.append("--use_fetch_service")
 args.append(self.name)
 self.runTargetSubProcess("buildsnap", *args)
 
diff --git a/lpbuildd/target/build_snap.py b/lpbuildd/target/build_snap.py
index d9052f2..d649fad 100644
--- a/lpbuildd/target/build_snap.py
+++ b/lpbuildd/target/build_snap.py
@@ -102,6 +102,12 @@ class BuildSnap(
 action="store_true",
 help="disable proxy access after the pull phase has finished",
 )
+parser.add_argument(
+"--use_fetch_service",
+default=False,
+action="store_true",
+help="use the fetch service instead of the old builder proxy",
+)
 parser.add_argument("name", help="name of snap to build")
 
 def install_svn_servers(self):
@@ -231,7 +237,9 @@ class BuildSnap(
 logger.info("Revoking proxy token...")
 try:
 revoke_proxy_token(
-self.args.upstream_proxy_url, self.args.revocation_endpoint
+self.args.upstream_proxy_url,
+self.args.revocation_endpoint,
+self.args.use_fetch_service,
 )
 except RevokeProxyTokenError as e:
 logger.info(str(e))
diff --git a/lpbuildd/tests/test_snap.py b/lpbuildd/tests/test_snap.py
index 9bef60a..fb72924 100644
--- a/lpbuildd/tests/test_snap.py
+++ b/lpbuildd/tests/test_snap.py
@@ -754,6 +754,13 @@ class TestSnapBuildManagerIteration(TestCase):
 yield self.startBuild(args, expected_options)
 
 @defer.inlineCallbacks
+def test_iterate_use_fetch_service(self):
+# The build manager can be told to use the fetch service as its proxy.
+args = {"use_fetch_service": True}
+ex

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

Given we will have 2 endpoints (one for token revocation, another for session 
closing), I decided to separate this into 2 MPs:
 - This one will handle adding the logic to call the revocation endpoint to the 
fetch service
 - The next one will handle closing the fetch service session and saving the 
data (to be opened)
-- 
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master has been updated.

Status: Needs review => Work in progress

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

In a meeting, decided that we will most likely need to have 2 endpoints for 
ending the session:
 - One for revoking the token
 - Another one to close the session

This MP is blocked until we settle that out.
-- 
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.

Commit message:
WIP: Update end-of-session code for snaps that use the fetch service

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875

This is WIP code with some TODOs and open questions to answer.
The goal is to get early feedback and direction.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into 
launchpad-buildd:master.
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py
index ea33dda..5b3d576 100644
--- a/lpbuildd/proxy.py
+++ b/lpbuildd/proxy.py
@@ -3,6 +3,7 @@
 
 import base64
 import io
+import os
 from urllib.parse import urlparse
 
 from twisted.application import strports
@@ -214,8 +215,21 @@ class BuilderProxyFactory(http.HTTPFactory):
 
 
 class BuildManagerProxyMixin:
+
+@property
+def _use_fetch_service(self):
+return (
+hasattr(self, 'use_fetch_service')
+and getattr(self, "use_fetch_service")
+)
+
 def startProxy(self):
 """Start the local builder proxy, if necessary."""
+
+if self._use_fetch_service:
+# TODO what to do if it is using the fetch service?
+return []
+
 if not self.proxy_url:
 return []
 proxy_port = self._builder._config.get("builder", "proxyport")
@@ -232,6 +246,11 @@ class BuildManagerProxyMixin:
 
 def stopProxy(self):
 """Stop the local builder proxy, if necessary."""
+
+if self._use_fetch_service:
+# TODO what to do if it is using the fetch service?
+return
+
 if self.proxy_service is None:
 return
 self.proxy_service.disownServiceParent()
@@ -243,6 +262,21 @@ class BuildManagerProxyMixin:
 return
 self._builder.log("Revoking proxy token...\n")
 try:
-revoke_proxy_token(self.proxy_url, self.revocation_endpoint)
+response = revoke_proxy_token(
+self.proxy_url,
+self.revocation_endpoint,
+self._use_fetch_service,
+)
 except RevokeProxyTokenError as e:
 self._builder.log(f"{e}\n")
+return
+
+# When using the fetch service, we want to retrieve and store the
+# metadata we receive at the end of the session.
+# This file will be uploaded when we run the `gatherResults` method
+if self._use_fetch_service:
+output_path = os.path.join(
+"/build", self.name, "fetch-service-metadata.json"
+)
+with self.backend.open(output_path, mode="wb") as f:
+f.write(response.content)
diff --git a/lpbuildd/snap.py b/lpbuildd/snap.py
index 02390d4..765bdbf 100644
--- a/lpbuildd/snap.py
+++ b/lpbuildd/snap.py
@@ -38,6 +38,7 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 self.branch = extra_args.get("branch")
 self.git_repository = extra_args.get("git_repository")
 self.git_path = extra_args.get("git_path")
+self.use_fetch_service = extra_args.get("use_fetch_service")
 self.proxy_url = extra_args.get("proxy_url")
 self.revocation_endpoint = extra_args.get("revocation_endpoint")
 self.build_source_tarball = extra_args.get(
@@ -100,6 +101,8 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 if self.target_architectures:
 for arch in self.target_architectures:
 args.extend(["--target-arch", arch])
+if self.use_fetch_service:
+args.append("--use_fetch_service")
 args.append(self.name)
 self.runTargetSubProcess("buildsnap", *args)
 
@@ -141,7 +144,14 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
 # `.comp` files are the binary result of building snap
 # components, see spec SD149.
 if entry.endswith(
-(".snap", ".manifest", ".debug", ".dpkg.yaml", ".comp")
+(
+".snap",
+".manifest",
+".debug",
+".dpkg.yaml",
+".comp",
+".json",
+)
 ):
 self.addWaitingFileFromBackend(path)
 if 

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:bump-launchpad-layers into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging ~ines-almeida/launchpad:bump-launchpad-layers 
into launchpad:master.

Commit message:
charms: bump launchpad-layers to 910dfb76754add5ed032a243c6081ee6a6b11256

The new source-commit updates the base configuration values for git related 
domains so that the git URLs exist even if the environment doesn't have git 
code hosting set up - which was causing issues in staging.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/462515

Related to: 
https://code.launchpad.net/~ines-almeida/launchpad-layers/+git/launchpad-layers/+merge/462428
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:bump-launchpad-layers into launchpad:master.
diff --git a/charm/launchpad-admin/charmcraft.yaml b/charm/launchpad-admin/charmcraft.yaml
index 3fa734d..bac0db1 100644
--- a/charm/launchpad-admin/charmcraft.yaml
+++ b/charm/launchpad-admin/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-appserver/charmcraft.yaml b/charm/launchpad-appserver/charmcraft.yaml
index 607181d..8cd8c6a 100644
--- a/charm/launchpad-appserver/charmcraft.yaml
+++ b/charm/launchpad-appserver/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-assets/charmcraft.yaml b/charm/launchpad-assets/charmcraft.yaml
index 5d3fc15..6a7ee47 100644
--- a/charm/launchpad-assets/charmcraft.yaml
+++ b/charm/launchpad-assets/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-buildd-manager/charmcraft.yaml b/charm/launchpad-buildd-manager/charmcraft.yaml
index 3fa734d..bac0db1 100644
--- a/charm/launchpad-buildd-manager/charmcraft.yaml
+++ b/charm/launchpad-buildd-manager/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-codehosting/charmcraft.yaml b/charm/launchpad-codehosting/charmcraft.yaml
index 57c980a..3d16c55 100644
--- a/charm/launchpad-codehosting/charmcraft.yaml
+++ b/charm/launchpad-codehosting/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-copy-archive-publisher/charmcraft.yaml b/charm/launchpad-copy-archive-publisher/charmcraft.yaml
index 605f876..254fc3f 100644
--- a/charm/launchpad-copy-archive-publisher/charmcraft.yaml
+++ b/charm/launchpad-copy-archive-publisher/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-db-update/charmcraft.yaml b/charm/launchpad-db-update/charmcraft.yaml
index 3fa734d..bac0db1 100644
--- a/charm/launchpad-db-update/charmcraft.yaml
+++ b/charm/launchpad-db-update/charmcraft.yaml
@@ -35,7 +35,7 @@ parts:
 after:
   - ols-layers
 source: https://git.launchpad.net/launchpad-layers
-source-commit: "0fd04056ad0ddb366be6ad7d50991efed385ec48"
+source-commit: "910dfb76754add5ed032a243c6081ee6a6b11256"
 source-submodules: []
 source-type: git
 plugin: dump
diff --git a/charm/launchpad-debian-importer/charmcraft.yaml b/charm/launchpad-debian-importer/charmcraft.yaml
index 3fa734d..bac0db1 100644
--- a/charm/launchpad-debian-importer/charmcraft.yaml
+++ b/charm/launchpad-debian-importer/charmcr

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-ftpmaster-generate-content-cron-start into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:update-ftpmaster-generate-content-cron-start into 
launchpad:master.

Commit message:
charm: update ftpmaster-publisher genrate-content cron script to run earlier

This is relevant because currently this job runs during most Launchpad 
developer's working hours, and can block DB deployments.
Updating the start time of the script to start daily at 9pm instead of 4am


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/462433

This change was manually applied in production last week.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-ftpmaster-generate-content-cron-start into 
launchpad:master.
diff --git a/charm/launchpad-ftpmaster-publisher/templates/crontab.j2 b/charm/launchpad-ftpmaster-publisher/templates/crontab.j2
index 87b4237..2cb676c 100644
--- a/charm/launchpad-ftpmaster-publisher/templates/crontab.j2
+++ b/charm/launchpad-ftpmaster-publisher/templates/crontab.j2
@@ -5,7 +5,7 @@ LPCONFIG=launchpad-ftpmaster-publisher
 
 {% if active -%}
 03-58/5 * * * * umask 022; {{ code_dir }}/cronscripts/publish-ftpmaster.py -v -d ubuntu >> {{ logs_dir }}/publish-ftpmaster.log 2>&1
-02 4 * * * {{ code_dir }}/cronscripts/generate-contents-files.py -v --distribution=ubuntu >> {{ logs_dir }}/generate-contents-files.log 2>&1
+02 21 * * * {{ code_dir }}/cronscripts/generate-contents-files.py -v --distribution=ubuntu >> {{ logs_dir }}/generate-contents-files.log 2>&1
 
 # cprov 2008-02-07: parallel run of death-row, at :10 it will run simultaneously with apt-ftparchive.
 10 */6 * * * {{ code_dir }}/scripts/process-death-row.py -d ubuntu -q --log-file=DEBUG:{{ logs_dir }}/process-death-row.log
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-layers:update-git-domain-config into launchpad-layers:main

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-layers:update-git-domain-config into 
launchpad-layers:main.

Commit message:
launchpad-base: update git domain configuration values to use default value if 
not set

`domain_git` is generally used to determine if an environment has git code 
hosting enabled or not.
But in cases where we don't want git hosting to be enabled, we still want the 
domain values to build and display URLs.
Adding a default placeholder value will enable us to display these placeholder 
URLs.



Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-layers/+git/launchpad-layers/+merge/462428

This replaces: 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/launchpad-mojo-specs/+merge/461985
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad-layers:update-git-domain-config into 
launchpad-layers:main.
diff --git a/launchpad-base/templates/launchpad-base-lazr.conf b/launchpad-base/templates/launchpad-base-lazr.conf
index a73f081..be77d35 100644
--- a/launchpad-base/templates/launchpad-base-lazr.conf
+++ b/launchpad-base/templates/launchpad-base-lazr.conf
@@ -34,11 +34,14 @@ lp_url_hosts: {{ bzr_lp_url_hosts }}
 secure_codebrowse_root: https://{{ domain_bzr }}/
 supermirror_root: http://{{ domain_bzr }}/
 {%- endif %}
+{#- If `domain_git` is not set, use a placeholder default value. #}
+{#- These domain config values are used to build URLs within Launchpad. #}
+{% set domain_git_or_default = domain_git or "git.not.enabled" %}
+internal_git_api_endpoint: http://{{ domain_git_or_default }}:19417/
+git_anon_root: git://{{ domain_git_or_default }}/
+git_browse_root: https://{{ domain_git_or_default }}/
+git_ssh_root: git+ssh://{{ domain_git_or_default }}/
 {%- if domain_git %}
-internal_git_api_endpoint: http://{{ domain_git }}:19417/
-git_anon_root: git://{{ domain_git }}/
-git_browse_root: https://{{ domain_git }}/
-git_ssh_root: git+ssh://{{ domain_git }}/
 {{- opt("loose_objects_threshold", git_loose_objects_threshold) }}
 {{- opt("packs_threshold", git_packs_threshold) }}
 {%- endif %}
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-refactor into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-option-refactor into launchpad:master.

Commit message:
Refactor snap.use_fetch_service tests and feature flag description

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/462338

This MP addressed the remaining comments from this other MP: 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461552
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-refactor into launchpad:master.
diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
index ae0bf52..3089b66 100644
--- a/lib/lp/services/features/flags.py
+++ b/lib/lp/services/features/flags.py
@@ -308,8 +308,8 @@ flag_info = sorted(
 "snap.fetch_service.enable",
 "boolean",
 "If set, allow admins to set snap.use_fetch_service field, which "
-"sets a snap build to use the fetch-service instead of the "
-"builder-proxy",
+"sets up the builds of a snap to use the fetch-servic instead of "
+"the builder-proxy",
 "",
 "",
 "",
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 07fc8d2..d18d2d1 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -2358,11 +2358,8 @@ class TestSnapSet(TestCaseWithFactory):
 self.assertEqual(ref.path, snap.git_path)
 self.assertEqual(ref, snap.git_ref)
 
-def test_auth_to_edit_admin_only_fields(self):
-# The admin fields can only be updated by an admin
-self.useFixture(
-FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
-)
+def test_non_admins_cannot_update_admin_only_fields(self):
+# The admin fields can not be updated by a non-admin
 
 non_admin = self.factory.makePerson()
 [ref] = self.factory.makeGitRefs(owner=non_admin)
@@ -2382,6 +2379,25 @@ class TestSnapSet(TestCaseWithFactory):
 self.assertRaises(
 Unauthorized, setattr, snap, field_name, True
 )
+
+def test_admins_can_update_admin_only_fields(self):
+# The admin fields can be updated by an admin
+self.useFixture(
+FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
+)
+
+[ref] = self.factory.makeGitRefs()
+components = self.makeSnapComponents(git_ref=ref)
+snap = getUtility(ISnapSet).new(**components)
+
+admin_fields = [
+"allow_internet",
+"pro_enable",
+"require_virtualized",
+"use_fetch_service",
+]
+
+for field_name in admin_fields:
 # exception isn't raised when an admin does the same
 with admin_logged_in():
 setattr(snap, field_name, True)
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-unit-test-fix into launchpad:master

[Ines Almeida]

Relevant test failure log: 
http://lpbuildbot.canonical.com/builders/lp-devel-focal/builds/504/steps/shell/logs/summary
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/462153
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-unit-test-fix into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-unit-test-fix into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-option-unit-test-fix into 
launchpad:master.

Commit message:
test: ensure feature flag is ON during test_admin_snap unit test


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/462153

This was not an issue when the UI MP was created because the 
`snap.use_fetch_service` was not hidden behind the feature flag.

Since we decided to hide it behind the feature flag in the API MP, this test 
fails because the feature flag is apparently "OFF" by the end of the test, by 
some odd reason.

This seems to be specifically a test set up/dependencies issue, that we'll need 
to look further into.
Currently, this is blocking other work, so my proposition is to simply ensure 
the feature flag after the browser functions run, and fix the inherent issue as 
a different task (https://warthogs.atlassian.net/browse/LP-1494).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-unit-test-fix into 
launchpad:master.
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 1127a50..8d2a48a 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -857,6 +857,12 @@ class TestSnapAdminView(BaseTestSnapView):
 browser.getControl("Use fetch service").selected = True
 browser.getControl("Update snap package").click()
 
+# XXX ines-almeida 2024-03-11: Browser tests work oddly with fixtures.
+# This ensures that the feature flag is ON during the rest of the test.
+# Further investigation on this issue is required.
+self.useFixture(
+FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: True})
+)
 login_admin()
 self.assertEqual(project, snap.project)
 self.assertFalse(snap.require_virtualized)
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
~launchpad/launchpad-mojo-specs/+git/private:master.

Commit message:
lp-fetch-service: add num_units to bundle

Requested reviews:
  Canonical Launchpad Engineering (launchpad)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461806

This was missing from the last MP. Without this, there are no units initiated
-- 
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.
diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
index e23fe8a..f61625e 100644
--- a/lp-fetch-service/bundle.yaml
+++ b/lp-fetch-service/bundle.yaml
@@ -19,4 +19,5 @@ applications:
 charm: "./fetch-service_ubuntu-22.04-amd64.charm"
 resources:
   snap: "./fetch-service.snap"
+num_units: 1
 {%- endif %}
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master has been updated.

Status: Needs review => Superseded

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461805
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master.

Commit message:
lp-fetch-service: add num_units to bundle

Requested reviews:
  Canonical Launchpad Engineering (launchpad)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461805

This was missing from the last MP. Without this, there are no units initiated
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.
diff --git a/lp-fetch-service/README.md b/lp-fetch-service/README.md
new file mode 100644
index 000..1ad35cd
--- /dev/null
+++ b/lp-fetch-service/README.md
@@ -0,0 +1,21 @@
+# Launchpad fetch service
+
+This spec deploys Launchpad's fetch service.
+
+You can run it locally using Juju's LXD support and Mojo. First, configure
+your environment:
+
+export MOJO_ROOT="$HOME/.local/share/mojo"
+export MOJO_PROJECT=mojo-lp-fetch-service
+export MOJO_WORKSPACE=devel
+export MOJO_SERIES=jammy
+export MOJO_SPEC="$HOME/spec"
+export MOJO_STAGE=lp-fetch-service/devel
+
+Then run the spec using Mojo:
+
+mojo project-new -c containerless
+mojo workspace-new
+mojo run
+
+You must have python3-yaml installed.
diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
new file mode 100644
index 000..f61625e
--- /dev/null
+++ b/lp-fetch-service/bundle.yaml
@@ -0,0 +1,23 @@
+{%- if stage_name == "production" %}
+{%-   set devel = False %}
+{%- elif stage_name == "qastaging" %}
+{%-   set devel = False %}
+{%- else %}
+{%-   set devel = True %}
+{%- endif -%}
+
+series: jammy
+applications:
+{%- if devel or stage_name == "qastaging" %}
+  fetch-service:
+{#- While the fetch-service charm and snap are not public, we are deploying
+a locally built charm. The lines below should be replaced with the path
+to charmhub, channel and revision number once the charm and snap are
+public. Currently, this expects the charm and snap to be copied to the
+path where the bundle.yaml is rendered, in the mojo project folder (eg.
+.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
+charm: "./fetch-service_ubuntu-22.04-amd64.charm"
+resources:
+  snap: "./fetch-service.snap"
+num_units: 1
+{%- endif %}
diff --git a/lp-fetch-service/configs/custom-secgroups-staging.yaml b/lp-fetch-service/configs/custom-secgroups-staging.yaml
new file mode 100644
index 000..78f1546
--- /dev/null
+++ b/lp-fetch-service/configs/custom-secgroups-staging.yaml
@@ -0,0 +1,9 @@
+applications:
+fetch-service:
+type: neutron
+rules:
+- rsync-logs
+rules:
+rsync-logs:
+# Allow launchpad-bastion-ps5 to fetch logs.
+- {"protocol": "tcp", "family": "IPv4", "port": 873, "cidr": "10.131.10.100/32"}
diff --git a/lp-fetch-service/manifest b/lp-fetch-service/manifest
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-perform-autodeploy b/lp-fetch-service/manifest-perform-autodeploy
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest-perform-autodeploy
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-verify b/lp-fetch-service/manifest-verify
new file mode 12
index 000..6e02de4
--- /dev/null
+++ b/lp-fetch-service/manifest-verify
@@ -0,0 +1 @@
+manifests/verify
\ No newline at end of file
diff --git a/lp-fetch-service/manifests/deploy b/lp-fetch-service/manifests/deploy
new file mode 100644
index 000..2d52196
--- /dev/null
+++ b/lp-fetch-service/manifests/deploy
@@ -0,0 +1,5 @@
+script config=predeploy
+bundle config=bundle.yaml max-wait=900 local=deploy-secrets
+juju-check-wait
+include config=manifests/secgroups
+include config=manifests/verify
diff --git a/lp-fetch-service/manifests/secgroups b/lp-fetch-service/manifests/secgroups
new file mode 100644
index 000..8c438ee
--- /dev/null
+++ b/lp-fetch-service/manifests/secgroups
@@ -0,0 +1 @@
+script config=utils/custom-secgroups.py SKIP_STAGES=devel
diff --git a/lp-fetch-service/manifests/verify b/lp-fetch-service/manifests/verify
new file mode 100644
index 000..f25f902
--- /dev/null
+++ b/lp-fetch-service/manifests/verify
@@ -0,0 +1,4 @@
+juju-check-wait
+# It occasionally takes a little while for all the servers to start
+# accepting connections.
+verify retry=3
diff --git a/lp-fetch-service/predeploy b/lp-fetch-service/predeploy
new file mode 100755
index 000..e6e2fbc
--- /dev/null
+++ b/lp-fetch-service/predeploy
@@ -0,0 +1,3 @@
+#! /bin/sh
+set -e
+exit 0
diff --git a/lp-fe

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

[Ines Almeida]

Diff comments:

> diff --git a/lib/lp/buildmaster/downloader.py 
> b/lib/lp/buildmaster/downloader.py
> index fc16852..cee308a 100644
> --- a/lib/lp/buildmaster/downloader.py
> +++ b/lib/lp/buildmaster/downloader.py
> @@ -94,3 +110,24 @@ class RequestProcess(AMPChild):
>  )
>  response.raise_for_status()
>  return response.json()
> +
> +@RequestFetchServiceSessionCommand.responder
> +def requestFetchServiceSessionCommand(
> +self, url, auth_header, proxy_username
> +):
> +with Session() as session:
> +session.trust_env = False
> +# XXX pelpsi 2024-02-28: should take the same

Will you ask before merging this MP? If so that's fine, but if we want to merge 
this MP first, then I'd clarify the comment itself so that a person that reads 
it understands what you just said above

> +# json body? From ST108 we should provide
> +# {
> +#   "timeout": ,// session timeout in 
> seconds
> +#   "policy":// "strict" or 
> "permissive"
> +# }
> +response = session.post(
> +url,
> +headers={"Authorization": auth_header},
> +json={"username": proxy_username},
> +)
> +print(" CI SIAMO ")
> +response.raise_for_status()
> +return response.json()
> diff --git a/lib/lp/buildmaster/tests/fetchservice.py 
> b/lib/lp/buildmaster/tests/fetchservice.py
> new file mode 100644
> index 000..9a12933
> --- /dev/null
> +++ b/lib/lp/buildmaster/tests/fetchservice.py
> @@ -0,0 +1,119 @@
> +# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the

You updated this to 2015-2024 but it should be 
```
 Copyright 2024 Canonical Ltd.
```

This is a new file (starting in 2024) and we no longer want to add the end date 
to these.

> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Fixtures for dealing with the build time HTTP proxy."""
> +
> +import json
> +import uuid
> +from textwrap import dedent
> +from urllib.parse import urlsplit
> +
> +import fixtures
> +from testtools.matchers import Equals, HasLength, MatchesStructure
> +from twisted.internet import defer, endpoints, reactor
> +from twisted.python.compat import nativeString
> +from twisted.web import resource, server
> +
> +from lp.services.config import config
> +
> +
> +class FetchServiceAuthAPITokensResource(resource.Resource):
> +"""A test tokens resource for the proxy authentication API."""
> +
> +isLeaf = True
> +
> +def __init__(self):
> +resource.Resource.__init__(self)
> +self.requests = []
> +
> +def render_POST(self, request):
> +content = json.loads(request.content.read().decode("UTF-8"))
> +self.requests.append(
> +{
> +"method": request.method,
> +"uri": request.uri,
> +"headers": dict(request.requestHeaders.getAllRawHeaders()),
> +"json": content,
> +}
> +)
> +return json.dumps(
> +{
> +"id": "1",
> +"token": uuid.uuid4().hex,
> +}
> +).encode("UTF-8")
> +
> +
> +class InProcessFetchServiceAuthAPIFixture(fixtures.Fixture):
> +"""A fixture that pretends to be the proxy authentication API.
> +
> +Users of this fixture must call the `start` method, which returns a
> +`Deferred`, and arrange for that to get back to the reactor.  This is
> +necessary because the basic fixture API does not allow `setUp` to return
> +anything.  For example:
> +
> +class TestSomething(TestCase):
> +
> +run_tests_with = AsynchronousDeferredRunTest.make_factory(
> +timeout=30)
> +
> +@defer.inlineCallbacks
> +def setUp(self):
> +super().setUp()
> +yield self.useFixture(
> +InProcessFetchServiceAuthAPIFixture()
> +).start()
> +"""
> +
> +@defer.inlineCallbacks
> +def start(self):
> +root = resource.Resource()
> +self.tokens = FetchServiceAuthAPITokensResource()
> +root.putChild(b"tokens", self.tokens)
> +endpoint = endpoints.serverFromString(reactor, nativeString("tcp:0"))
> +site = server.Site(self.tokens)
> +self.addCleanup(site.stopFactory)
> +port = yield endpoint.listen(site)
> +self.addCleanup(port.stopListening)
> +config.push(
> +"in-process-fetch-service-api-fixture",
> +dedent(
> +"""
> +[builddmaster]
> +fetch_service_auth_api_admin_secret: admin-secret
> +fetch_service_auth_api_admin_username: admin-launchpad.test
> +fetch_service_auth_api_endpoint: http://{host}:{port}/tokens
> +

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

[Ines Almeida]

Good work - left a few comments!
I'll leave the approval to Jürgen who is more into the details of the whole 
project.

Diff comments:

> diff --git a/lib/lp/buildmaster/builderproxy.py 
> b/lib/lp/buildmaster/builderproxy.py
> index be65e29..4f249a7 100644
> --- a/lib/lp/buildmaster/builderproxy.py
> +++ b/lib/lp/buildmaster/builderproxy.py
> @@ -52,6 +62,25 @@ class BuilderProxyMixin:
>  
> endpoint=_get_proxy_config("builder_proxy_auth_api_endpoint"),
>  token=token["username"],
>  )
> +if (

This should be an `elif` so that it doesn't try to check the second `if 
statement if the first is true.

Also, how about doing:
```
if not allow_internet:
return

if fetch_service and _get_proxy_config("fetch_service_host"):
...
elif not fetch_service and _get_proxy_config("builder_proxy_host"):
...
```

Seems a bit easier to follow IMO

> +fetch_service
> +and _get_proxy_config("fetch_service_host")
> +and allow_internet
> +):
> +# http://:@:9988/

Can you be more clear with what you mean with the comment? Is this the expected 
proxy_url format? If so, I see the string format bellow already hints the 
format quite well, so perhaps this is not needed

> +token = yield self._requestFetchServiceSession()
> +args["proxy_url"] = (
> +"http://{session_id}:{token}@{host}:{port}".format(
> +session_id=token["id"],
> +token=token["token"],
> +host=_get_proxy_config("fetch_service_host"),
> +port=_get_proxy_config("fetch_service_port"),
> +)
> +)
> +args["revocation_endpoint"] = "{endpoint}/session/{id}".format(
> +
> endpoint=_get_proxy_config("fetch_service_auth_api_endpoint"),
> +id=token["id"],
> +)
>  
>  @defer.inlineCallbacks
>  def _requestProxyToken(self):
> @@ -86,3 +115,39 @@ class BuilderProxyMixin:
>  proxy_username=proxy_username,
>  )
>  return token
> +
> +@defer.inlineCallbacks
> +def _requestFetchServiceSession(self):
> +# This is a different function if we have the needs to

Not sure what you mean in this comment, can you clarify?

> +# differentiate more the behavior.
> +admin_username = _get_proxy_config(
> +"fetch_service_auth_api_admin_username"
> +)
> +if not admin_username:

Since this pattern is used so much in this class here and by the builder proxy 
route (checking a variable and raising), perhaps we could have a method to 
check values are in config and raise separately?

Something like:
```
def get_value_from_config(self, key: string):
value = _get_proxy_config(key)
if not value:
raise CannotBuild(f"'{key}' is not configured")
return value
```

Which could then be called as:
```
admin_username = 
self.get_value_from_config("fetch_service_auth_api_admin_username")
```

Just a thought, this is also fine as is.

> +raise CannotBuild(
> +"fetch_service_auth_api_admin_username is not configured."
> +)
> +secret = _get_proxy_config("fetch_service_auth_api_admin_secret")
> +if not secret:
> +raise CannotBuild(
> +"fetch_service_auth_api_admin_secret is not configured."
> +)
> +url = _get_proxy_config("fetch_service_auth_api_endpoint")
> +if not secret:
> +raise CannotBuild(
> +"fetch_service_auth_api_endpoint is not configured."
> +)
> +timestamp = int(time.time())
> +proxy_username = "{build_id}-{timestamp}".format(
> +build_id=self.build.build_cookie, timestamp=timestamp
> +)
> +auth_string = f"{admin_username}:{secret}".strip()
> +auth_header = b"Basic " + 
> base64.b64encode(auth_string.encode("ASCII"))
> +
> +token = yield self._worker.process_pool.doWork(
> +RequestFetchServiceSessionCommand,
> +url=url,
> +auth_header=auth_header,
> +proxy_username=proxy_username,
> +)
> +return token
> diff --git a/lib/lp/buildmaster/downloader.py 
> b/lib/lp/buildmaster/downloader.py
> index fc16852..cee308a 100644
> --- a/lib/lp/buildmaster/downloader.py
> +++ b/lib/lp/buildmaster/downloader.py
> @@ -94,3 +110,24 @@ class RequestProcess(AMPChild):
>  )
>  response.raise_for_status()
>  return response.json()
> +
> +@RequestFetchServiceSessionCommand.responder
> +def requestFetchServiceSessionCommand(
> +self, url, auth_header, proxy_username
> +):
> +with Session() as session:
> +session.trust_env = False
> +# XXX pelpsi 2024-02-28: should take the same

I'm not sure I understand what's missing by this comment, can 

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-model-update into launchpad:master

[Ines Almeida]

The proposal to merge ~ines-almeida/launchpad:fetch-service-option-model-update 
into launchpad:master has been updated.

Description changed to:

UI changes in another MP: 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461649

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461552
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-model-update into launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-ui-update into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-option-ui-update into launchpad:master 
with ~ines-almeida/launchpad:fetch-service-option-model-update as a 
prerequisite.

Commit message:
UI changes to allow admins to update a Snap's use_fetch_service

This is hidden behind the new snap.fetch_service.enable feature flag



Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461649

Related to: 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461552
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-ui-update into launchpad:master.
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index 5744565..dc4d330 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -77,6 +77,7 @@ from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget
 from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget
 from lp.snappy.interfaces.snap import (
 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
+SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
 CannotAuthorizeStoreUploads,
 CannotFetchSnapcraftYaml,
 CannotParseSnapcraftYaml,
@@ -525,6 +526,7 @@ class ISnapEditSchema(Interface):
 "auto_build_channels",
 "store_upload",
 "pro_enable",
+"use_fetch_service",
 ],
 )
 
@@ -929,13 +931,20 @@ class SnapAdminView(BaseSnapEditView):
 # XXX pappacena 2021-02-19: Once we have the whole privacy work in
 # place, we should move "project" and "information_type" from +admin
 # page to +edit, to allow common users to edit this.
-field_names = [
-"project",
-"information_type",
-"require_virtualized",
-"allow_internet",
-"pro_enable",
-]
+@property
+def field_names(self):
+fields = [
+"project",
+"information_type",
+"require_virtualized",
+"allow_internet",
+"pro_enable",
+]
+
+if getFeatureFlag(SNAP_USE_FETCH_SERVICE_FEATURE_FLAG):
+fields.append("use_fetch_service")
+
+return fields
 
 @property
 def initial_values(self):
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index cb4d4de..1127a50 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -61,6 +61,7 @@ from lp.snappy.browser.snap import (
 )
 from lp.snappy.interfaces.snap import (
 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
+SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
 CannotModifySnapProcessor,
 ISnapSet,
 SnapBuildRequestStatus,
@@ -821,6 +822,11 @@ class TestSnapAdminView(BaseTestSnapView):
 
 def test_admin_snap(self):
 # Admins can change require_virtualized, privacy, and allow_internet.
+
+self.useFixture(
+FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: True})
+)
+
 login("ad...@canonical.com")
 admin = self.factory.makePerson(
 member_of=[getUtility(ILaunchpadCelebrities).admin]
@@ -835,6 +841,7 @@ class TestSnapAdminView(BaseTestSnapView):
 self.assertFalse(snap.private)
 self.assertTrue(snap.allow_internet)
 self.assertFalse(snap.pro_enable)
+self.assertFalse(snap.use_fetch_service)
 
 self.factory.makeAccessPolicy(
 pillar=project, type=InformationType.PRIVATESECURITY
@@ -847,6 +854,7 @@ class TestSnapAdminView(BaseTestSnapView):
 browser.getControl(name="field.information_type").value = private
 browser.getControl("Allow external network access").selected = False
 browser.getControl("Enable Ubuntu Pro").selected = True
+browser.getControl("Use fetch service").selected = True
 browser.getControl("Update snap package").click()
 
 login_admin()
@@ -855,6 +863,24 @@ class TestSnapAdminView(BaseTestSnapView):
 self.assertTrue(snap.private)
 self.assertFalse(snap.allow_internet)
 self.assertTrue(snap.pro_enable)
+self.assertTrue(snap.use_fetch_service)
+
+def test_admin_use_fetch_service_feature_flag(self):
+admin = self.factory.makePerson(
+member_of=[getUtility(ILaunchpadCelebrities).admin]
+)
+snap = self.factory.makeSnap(registrant=admin)
+browser = self.getViewBrowser(snap, user=admin)
+
+browser.getLink("Administer snap package").click()
+self.assertFalse("Use fetch service" in browser.contents)
+
+

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-model-update into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-option-model-update into launchpad:master.

Commit message:
Add Snap.use_fetch_service field to model and API

The field will only be updatable by admins. Although we can't hide the API 
endpoint itself, we are hidding the endpoint setting and getting behind a new 
feature flag "snap.fetch_service.enable"

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461552
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-model-update into launchpad:master.
diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
index f15a6df..ae0bf52 100644
--- a/lib/lp/services/features/flags.py
+++ b/lib/lp/services/features/flags.py
@@ -304,6 +304,16 @@ flag_info = sorted(
 "",
 "",
 ),
+(
+"snap.fetch_service.enable",
+"boolean",
+"If set, allow admins to set snap.use_fetch_service field, which "
+"sets a snap build to use the fetch-service instead of the "
+"builder-proxy",
+"",
+"",
+"",
+),
 ]
 )
 
diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index ae2e3df..8364c58 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -23,6 +23,7 @@ __all__ = [
 "NoSourceForSnap",
 "NoSuchSnap",
 "SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG",
+"SNAP_USE_FETCH_SERVICE_FEATURE_FLAG",
 "SnapAuthorizationBadGeneratedMacaroon",
 "SnapBuildAlreadyPending",
 "SnapBuildArchiveOwnerMismatch",
@@ -101,6 +102,7 @@ from lp.soyuz.interfaces.archive import IArchive
 from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
 
 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG = "snap.channels.snapcraft"
+SNAP_USE_FETCH_SERVICE_FEATURE_FLAG = "snap.fetch_service.enable"
 
 
 @error_status(http.client.BAD_REQUEST)
@@ -1189,6 +1191,18 @@ class ISnapAdminAttributes(Interface):
 )
 )
 
+use_fetch_service = exported(
+Bool(
+title=_("Use fetch service"),
+required=True,
+readonly=False,
+description=_(
+"If set, Snap builds will use the fetch-service instead "
+"of the builder-proxy to access external resources."
+),
+)
+)
+
 def subscribe(person, subscribed_by):
 """Subscribe a person to this snap recipe."""
 
@@ -1267,6 +1281,7 @@ class ISnapSet(Interface):
 store_channels=None,
 project=None,
 pro_enable=None,
+use_fetch_service=False,
 ):
 """Create an `ISnap`."""
 
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index eeeb056..cf18262 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -124,6 +124,7 @@ from lp.services.database.stormexpr import (
 IsDistinctFrom,
 NullsLast,
 )
+from lp.services.features import getFeatureFlag
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.model.job import Job
 from lp.services.librarian.model import LibraryFileAlias, LibraryFileContent
@@ -137,6 +138,7 @@ from lp.services.webhooks.interfaces import IWebhookSet
 from lp.services.webhooks.model import WebhookTargetMixin
 from lp.snappy.adapters.buildarch import determine_architectures_to_build
 from lp.snappy.interfaces.snap import (
+SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
 BadMacaroon,
 BadSnapSearchContext,
 BadSnapSource,
@@ -394,6 +396,8 @@ class Snap(StormBase, WebhookTargetMixin):
 
 _pro_enable = Bool(name="pro_enable", allow_none=True)
 
+_use_fetch_service = Bool(name="use_fetch_service", allow_none=False)
+
 def __init__(
 self,
 registrant,
@@ -419,6 +423,7 @@ class Snap(StormBase, WebhookTargetMixin):
 store_channels=None,
 project=None,
 pro_enable=False,
+use_fetch_service=False,
 ):
 """Construct a `Snap`."""
 super().__init__()
@@ -454,6 +459,7 @@ class Snap(StormBase, WebhookTargetMixin):
 self.store_secrets = store_secrets
 self.store_channels = store_channels
 self._pro_enable = pro_enable
+self.use_fetch_service = use_fetch_service
 
 def __repr__(self):
 return "" % (self.owner.name, self.name)
@@ -693,6 +699,17 @@ class Snap(StormBase, WebhookTargetMixin):

[Launchpad-reviewers] [Merge] ~ines-almeida/lp-source-dependencies:revert-pyyaml-dependency-deletion into lp-source-dependencies:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/lp-source-dependencies:revert-pyyaml-dependency-deletion into 
lp-source-dependencies:master.

Commit message:
Revert "Remove PyYAML 5.3.1 and 5.4.1"

This reverts commit af9c3a1b91538b0488e7ca57f9bba86fb4b53741.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/lp-source-dependencies/+git/lp-source-dependencies/+merge/461524
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/lp-source-dependencies:revert-pyyaml-dependency-deletion into 
lp-source-dependencies:master.
diff --git a/dist/PyYAML-5.3.1.tar.gz b/dist/PyYAML-5.3.1.tar.gz
new file mode 100644
index 000..915d67b
Binary files /dev/null and b/dist/PyYAML-5.3.1.tar.gz differ
diff --git a/dist/PyYAML-5.4.1.tar.gz b/dist/PyYAML-5.4.1.tar.gz
new file mode 100644
index 000..187c66e
Binary files /dev/null and b/dist/PyYAML-5.4.1.tar.gz differ
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-db-update into launchpad:db-devel

[Ines Almeida]

Lint issues don't seem related to this MP. I'll take a look into it tomorrow
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461264
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-db-update into launchpad:db-devel.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

I'll go ahead and merge this MP, having in mind that it might still require 
some changes to get it to the place we need it to be. Such changes can be 
handled in follow-up MPs.
-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461232
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:fetch-service-option-db-update into launchpad:db-devel

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:fetch-service-option-db-update into launchpad:db-devel.

Commit message:
db: add new field Snap.use_fetch_service

The new field will dictate whether, while building a snap, the builder will use 
the (new) fetch-service or the (current) builder-proxy for external access


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): db

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461264

Related to: https://warthogs.atlassian.net/browse/LP-1461
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:fetch-service-option-db-update into launchpad:db-devel.
diff --git a/database/schema/patch-2211-26-0.sql b/database/schema/patch-2211-26-0.sql
new file mode 100644
index 000..6f60d7e
--- /dev/null
+++ b/database/schema/patch-2211-26-0.sql
@@ -0,0 +1,10 @@
+-- Copyright 2024 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+ALTER TABLE Snap ADD COLUMN use_fetch_service boolean DEFAULT false NOT NULL;
+
+COMMENT ON COLUMN Snap.use_fetch_service IS 'Whether to use the fetch-service in place of the builder-proxy when building this snap.';
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2211, 26, 0);
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-fix-demo-background-staging into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:frontpage-revamp-fix-demo-background-staging into 
launchpad:master.

Commit message:
ui: ensure 'demo' background is set for (qa)staging environments

After the changes to the homepage, it is no longer displaying the 'demo' 
background in qastaging due to a 'background-repeat: no-repeat' setting. This 
ensures that when we want to show this backgroud, this setting is set to 
'repeat'.
The demo background makes it more obvious to users that this is a staging 
environment (not production)


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461260

See https://qastaging.launchpad.net/
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-fix-demo-background-staging into 
launchpad:master.
diff --git a/lib/lp/app/templates/base-layout.pt b/lib/lp/app/templates/base-layout.pt
index 74b4f45..4ddb539 100644
--- a/lib/lp/app/templates/base-layout.pt
+++ b/lib/lp/app/templates/base-layout.pt
@@ -49,7 +49,7 @@
   use-macro="context/@@+base-layout-macros/launchpad-stylesheet-3-0" />
 
 
-  html, body {background-image: url(/@@/demo) !important;}
+  html, body {background-image: url(/@@/demo) !important; background-repeat: repeat !important}
 
 
 
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
~launchpad/launchpad-mojo-specs/+git/private:master has been updated.

Commit message changed to:

lp-fetch-service: Add barebones of lp-fetch-service mojo spec

While the charm and snap are not public, they require manually building, and 
deploying from the local charm

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461232
-- 
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

Thank you for the review! Comments were addressed.
I also updated the commit message to mention the service to be consistent with 
other commit messages in this repo.

Diff comments:

> diff --git a/lp-fetch-service/README.md b/lp-fetch-service/README.md
> new file mode 100644
> index 000..65e09cb
> --- /dev/null
> +++ b/lp-fetch-service/README.md
> @@ -0,0 +1,21 @@
> +# Launchpad fetch service
> +
> +This spec deploys Launchpad's fetch service.
> +
> +You can run it locally using Juju's LXD support and Mojo. First, configure
> +your environment:
> +
> +export MOJO_ROOT="$HOME/.local/share/mojo"
> +export MOJO_PROJECT=mojo-lp-fetch-service
> +export MOJO_WORKSPACE=devel
> +export MOJO_SERIES=jammy
> +export 
> MOJO_SPEC=git+https://git.launchpad.net/~launchpad/launchpad-mojo-specs/+git/private

Ok makes sense. I have updated it to `$HOME/spec` and I'm guessing I'll have to 
create that local directory.

Hopefully this can eventually be made public and we can use the public git 
address here.

> +export MOJO_STAGE=lp-fetch-service/devel
> +
> +Then run the spec using Mojo:
> +
> +mojo project-new -c containerless
> +mojo workspace-new
> +mojo run
> +
> +You must have python3-yaml installed.
> diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
> new file mode 100644
> index 000..286
> --- /dev/null
> +++ b/lp-fetch-service/bundle.yaml
> @@ -0,0 +1,22 @@
> +{%- if stage_name == "production" %}
> +{%-   set devel = False %}
> +{%- elif stage_name == "staging" %}
> +{%-   set devel = False %}
> +{%- else %}
> +{%-   set devel = True %}
> +{%- endif -%}
> +
> +series: jammy
> +applications:
> +{%- if devel or stage_name == "staging" %}

Great point, updated!

> +  fetch-service:
> +{#- While the fetch-service charm and snap are not public, we are 
> deploying
> +a locally built charm. The lines below should be replaced with the 
> path
> +to charmhub, channel and revision number once the charm and snap are
> +public. Currently, this expects the charm and snap to be copied to 
> the
> +path where the bundle.yaml is rendered, in the mojo project folder 
> (eg.
> +.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
> +charm: "./fetch-service_ubuntu-22.04-amd64.charm"
> +resources:
> +  snap: "./fetch-service.snap"
> +{%- endif %}
> diff --git a/lp-fetch-service/verify b/lp-fetch-service/verify
> new file mode 100755
> index 000..df9e6ef
> --- /dev/null
> +++ b/lp-fetch-service/verify
> @@ -0,0 +1,8 @@
> +#! /bin/sh
> +set -e
> +
> +TOP="${0%/*}"
> +
> +export 
> EXTRA_SKIP_CHECKS="check_swap${EXTRA_SKIP_CHECKS:+|${EXTRA_SKIP_CHECKS}}"

My only reason why we want to do it is that it is skipped in all other specs.

I understand that that might not be a good enough answer here, but I can't 
really find the reason why it is like that in every other spec as I can't find 
any mentions of it within comments.

Some info about this check: https://nagios-plugins.org/doc/man/check_swap.html
Some info about swap space: https://help.ubuntu.com/community/SwapFaq

I'll try to investigate and add a comment in the spec with the information I 
find, but for now I'll keep the SKIP in the code

> +
> +exec "$TOP/utils/verify"


-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461232
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into ~launchpad/launchpad-mojo-specs/+git/private:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
~launchpad/launchpad-mojo-specs/+git/private:master.

Commit message:
Add barebones of lp-fetch-service mojo spec

While the charm and snap are not public, they require manually building, and 
deploying from the local charm


Requested reviews:
  Jürgen Gmach (jugmac00)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461232

Resubmitting this MP to update the target branch.


This was tested deploying locally. Will try deploying in staging once this MP 
is reviewed and approved.

The general structure was based on similar lp specs.


-- 
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:master.
diff --git a/lp-fetch-service/README.md b/lp-fetch-service/README.md
new file mode 100644
index 000..65e09cb
--- /dev/null
+++ b/lp-fetch-service/README.md
@@ -0,0 +1,21 @@
+# Launchpad fetch service
+
+This spec deploys Launchpad's fetch service.
+
+You can run it locally using Juju's LXD support and Mojo. First, configure
+your environment:
+
+export MOJO_ROOT="$HOME/.local/share/mojo"
+export MOJO_PROJECT=mojo-lp-fetch-service
+export MOJO_WORKSPACE=devel
+export MOJO_SERIES=jammy
+export MOJO_SPEC=git+https://git.launchpad.net/~launchpad/launchpad-mojo-specs/+git/private
+export MOJO_STAGE=lp-fetch-service/devel
+
+Then run the spec using Mojo:
+
+mojo project-new -c containerless
+mojo workspace-new
+mojo run
+
+You must have python3-yaml installed.
diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
new file mode 100644
index 000..286
--- /dev/null
+++ b/lp-fetch-service/bundle.yaml
@@ -0,0 +1,22 @@
+{%- if stage_name == "production" %}
+{%-   set devel = False %}
+{%- elif stage_name == "staging" %}
+{%-   set devel = False %}
+{%- else %}
+{%-   set devel = True %}
+{%- endif -%}
+
+series: jammy
+applications:
+{%- if devel or stage_name == "staging" %}
+  fetch-service:
+{#- While the fetch-service charm and snap are not public, we are deploying
+a locally built charm. The lines below should be replaced with the path
+to charmhub, channel and revision number once the charm and snap are
+public. Currently, this expects the charm and snap to be copied to the
+path where the bundle.yaml is rendered, in the mojo project folder (eg.
+.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
+charm: "./fetch-service_ubuntu-22.04-amd64.charm"
+resources:
+  snap: "./fetch-service.snap"
+{%- endif %}
diff --git a/lp-fetch-service/configs/custom-secgroups-staging.yaml b/lp-fetch-service/configs/custom-secgroups-staging.yaml
new file mode 100644
index 000..78f1546
--- /dev/null
+++ b/lp-fetch-service/configs/custom-secgroups-staging.yaml
@@ -0,0 +1,9 @@
+applications:
+fetch-service:
+type: neutron
+rules:
+- rsync-logs
+rules:
+rsync-logs:
+# Allow launchpad-bastion-ps5 to fetch logs.
+- {"protocol": "tcp", "family": "IPv4", "port": 873, "cidr": "10.131.10.100/32"}
diff --git a/lp-fetch-service/manifest b/lp-fetch-service/manifest
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-perform-autodeploy b/lp-fetch-service/manifest-perform-autodeploy
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest-perform-autodeploy
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-verify b/lp-fetch-service/manifest-verify
new file mode 12
index 000..6e02de4
--- /dev/null
+++ b/lp-fetch-service/manifest-verify
@@ -0,0 +1 @@
+manifests/verify
\ No newline at end of file
diff --git a/lp-fetch-service/manifests/deploy b/lp-fetch-service/manifests/deploy
new file mode 100644
index 000..2d52196
--- /dev/null
+++ b/lp-fetch-service/manifests/deploy
@@ -0,0 +1,5 @@
+script config=predeploy
+bundle config=bundle.yaml max-wait=900 local=deploy-secrets
+juju-check-wait
+include config=manifests/secgroups
+include config=manifests/verify
diff --git a/lp-fetch-service/manifests/secgroups b/lp-fetch-service/manifests/secgroups
new file mode 100644
index 000..8c438ee
--- /dev/null
+++ b/lp-fetch-service/manifests/secgroups
@@ -0,0 +1 @@
+script config=utils/custom-secgroups.py SKIP_STAGES=devel
diff --git a/lp-fetch-service/manifests/verify b/lp-fetch-service/manifests/verify
new file mode 100644
index 000..f25f902
--- /dev/null
+++ b/lp-fetch-service/manifests/verify
@@ -0,0 +1,4 @@
+juju-check-wait
+# It occasionally takes a little while for all the servers to start
+

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master has been updated.

Status: Needs review => Superseded

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461231
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master.

Commit message:
Add barebones of lp-fetch-service mojo spec

While the charm and snap are not public, they require manually building, and 
deploying from the local charm


Requested reviews:
  Jürgen Gmach (jugmac00)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461231

Resubmitting this MP to update the target branch.


This was tested deploying locally. Will try deploying in staging once this MP 
is reviewed and approved.

The general structure was based on similar lp specs.


-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.
diff --git a/lp-fetch-service/README.md b/lp-fetch-service/README.md
new file mode 100644
index 000..65e09cb
--- /dev/null
+++ b/lp-fetch-service/README.md
@@ -0,0 +1,21 @@
+# Launchpad fetch service
+
+This spec deploys Launchpad's fetch service.
+
+You can run it locally using Juju's LXD support and Mojo. First, configure
+your environment:
+
+export MOJO_ROOT="$HOME/.local/share/mojo"
+export MOJO_PROJECT=mojo-lp-fetch-service
+export MOJO_WORKSPACE=devel
+export MOJO_SERIES=jammy
+export MOJO_SPEC=git+https://git.launchpad.net/~launchpad/launchpad-mojo-specs/+git/private
+export MOJO_STAGE=lp-fetch-service/devel
+
+Then run the spec using Mojo:
+
+mojo project-new -c containerless
+mojo workspace-new
+mojo run
+
+You must have python3-yaml installed.
diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
new file mode 100644
index 000..286
--- /dev/null
+++ b/lp-fetch-service/bundle.yaml
@@ -0,0 +1,22 @@
+{%- if stage_name == "production" %}
+{%-   set devel = False %}
+{%- elif stage_name == "staging" %}
+{%-   set devel = False %}
+{%- else %}
+{%-   set devel = True %}
+{%- endif -%}
+
+series: jammy
+applications:
+{%- if devel or stage_name == "staging" %}
+  fetch-service:
+{#- While the fetch-service charm and snap are not public, we are deploying
+a locally built charm. The lines below should be replaced with the path
+to charmhub, channel and revision number once the charm and snap are
+public. Currently, this expects the charm and snap to be copied to the
+path where the bundle.yaml is rendered, in the mojo project folder (eg.
+.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
+charm: "./fetch-service_ubuntu-22.04-amd64.charm"
+resources:
+  snap: "./fetch-service.snap"
+{%- endif %}
diff --git a/lp-fetch-service/configs/custom-secgroups-staging.yaml b/lp-fetch-service/configs/custom-secgroups-staging.yaml
new file mode 100644
index 000..78f1546
--- /dev/null
+++ b/lp-fetch-service/configs/custom-secgroups-staging.yaml
@@ -0,0 +1,9 @@
+applications:
+fetch-service:
+type: neutron
+rules:
+- rsync-logs
+rules:
+rsync-logs:
+# Allow launchpad-bastion-ps5 to fetch logs.
+- {"protocol": "tcp", "family": "IPv4", "port": 873, "cidr": "10.131.10.100/32"}
diff --git a/lp-fetch-service/manifest b/lp-fetch-service/manifest
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-perform-autodeploy b/lp-fetch-service/manifest-perform-autodeploy
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest-perform-autodeploy
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-verify b/lp-fetch-service/manifest-verify
new file mode 12
index 000..6e02de4
--- /dev/null
+++ b/lp-fetch-service/manifest-verify
@@ -0,0 +1 @@
+manifests/verify
\ No newline at end of file
diff --git a/lp-fetch-service/manifests/deploy b/lp-fetch-service/manifests/deploy
new file mode 100644
index 000..2d52196
--- /dev/null
+++ b/lp-fetch-service/manifests/deploy
@@ -0,0 +1,5 @@
+script config=predeploy
+bundle config=bundle.yaml max-wait=900 local=deploy-secrets
+juju-check-wait
+include config=manifests/secgroups
+include config=manifests/verify
diff --git a/lp-fetch-service/manifests/secgroups b/lp-fetch-service/manifests/secgroups
new file mode 100644
index 000..8c438ee
--- /dev/null
+++ b/lp-fetch-service/manifests/secgroups
@@ -0,0 +1 @@
+script config=utils/custom-secgroups.py SKIP_STAGES=devel
diff --git a/lp-fetch-service/manifests/verify b/lp-fetch-service/manifests/verify
new file mode 100644
index 000..f25f902
--- /dev/null
+++ b/lp-fetch-service/manifests/verify
@@ -0,0 +1,4 @@
+juju-check-wait
+# It occasionally takes a little while for all the servers to start
+# acce

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master has been updated.

Status: Needs review => Superseded

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/460966
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-fix-test into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:frontpage-revamp-fix-test into launchpad:master.

Commit message:
tests: update unit test after logo update

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/461229
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-fix-test into launchpad:master.
diff --git a/lib/lp/app/browser/tests/test_launchpadroot.py b/lib/lp/app/browser/tests/test_launchpadroot.py
index 9d81273..dbcbda9 100644
--- a/lib/lp/app/browser/tests/test_launchpadroot.py
+++ b/lib/lp/app/browser/tests/test_launchpadroot.py
@@ -147,7 +147,7 @@ class LaunchpadRootIndexViewTestCase(TestCaseWithFactory):
 self.assertIs(None, markup.find(True, id="watermark"))
 logo = markup.find(True, id="launchpad-logo-and-name")
 self.assertIsNot(None, logo)
-self.assertEqual("/@@/launchpad-logo-and-name.png", logo["src"])
+self.assertEqual("/@@/launchpad-logo-and-name.svg", logo["src"])
 
 @staticmethod
 def _make_blog_post(linkid, title, body, date):
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

Diff comments:

> diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
> new file mode 100644
> index 000..286
> --- /dev/null
> +++ b/lp-fetch-service/bundle.yaml
> @@ -0,0 +1,22 @@
> +{%- if stage_name == "production" %}
> +{%-   set devel = False %}
> +{%- elif stage_name == "staging" %}
> +{%-   set devel = False %}
> +{%- else %}
> +{%-   set devel = True %}
> +{%- endif -%}
> +
> +series: jammy
> +applications:
> +{%- if devel or stage_name == "staging" %}

> It is probably just as straightforward as not to define the fetch service for 
> production?

That is exactly what the code is doing here: it only defines the fetch-service 
if `devel` (local) or `staging`.



I can't edit the comment above, but what it meant is that in the particular 
case that someone by accident deploys this mojo spec in production (which is 
not supposed to happen), it will throw an error because there is no application 
defined (i.e., the list if application when the "stage_name" is production is 
empty). But IMO it's OK to have that error

> +  fetch-service:
> +{#- While the fetch-service charm and snap are not public, we are 
> deploying
> +a locally built charm. The lines below should be replaced with the 
> path
> +to charmhub, channel and revision number once the charm and snap are
> +public. Currently, this expects the charm and snap to be copied to 
> the
> +path where the bundle.yaml is rendered, in the mojo project folder 
> (eg.
> +.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
> +charm: "./fetch-service_ubuntu-22.04-amd64.charm"
> +resources:
> +  snap: "./fetch-service.snap"
> +{%- endif %}


-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/460966
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-change-layout into ~ines-almeida/launchpad:frontpage-revamp-remove-top-project

[Ines Almeida]

I pushed a few new commits after the demo this morning:
 - The CSS file is now hosted locally
 - Small update to the style of  items in Launchpad
 - Updates the icon colors slightly and added a new icon
 - Updates the 'Launchpad' and 'Get Started' sections (see screenshots below)

Logged out: https://pasteboard.co/SgtcNkGLevV0.png
Logged in: https://pasteboard.co/j3yJPECBjUZY.png
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460819
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-change-layout into 
~ines-almeida/launchpad:frontpage-revamp-remove-top-project.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-change-layout into ~ines-almeida/launchpad:frontpage-revamp-remove-top-project

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:frontpage-revamp-change-layout into 
~ines-almeida/launchpad:frontpage-revamp-remove-top-project.

Commit message:
ui: Launchpad homepage revamp

 - Imported Vanilla framework directly to the homepage
 - Re-organized homepage sections and reworked them using Vanilla components
 - Cleaned up CSS styling and page views that are no longer used

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
  Peter Makowski (petermakowski): code

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460819

Initial stab at making Launchpad's homepage a little bit more modern
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-change-layout into 
~ines-almeida/launchpad:frontpage-revamp-remove-top-project.
diff --git a/lib/canonical/launchpad/icing/css/components/_index.scss b/lib/canonical/launchpad/icing/css/components/_index.scss
index 59e25ed..057da90 100644
--- a/lib/canonical/launchpad/icing/css/components/_index.scss
+++ b/lib/canonical/launchpad/icing/css/components/_index.scss
@@ -1,6 +1,7 @@
 @import 'access_tokens',
 'batch_navigation',
 'bug_picker',
+'homepage',
 'profiling_info',
 'sidebar_portlets',
 'bug_listing',
diff --git a/lib/canonical/launchpad/icing/css/components/homepage.scss b/lib/canonical/launchpad/icing/css/components/homepage.scss
new file mode 100644
index 000..17b63cc
--- /dev/null
+++ b/lib/canonical/launchpad/icing/css/components/homepage.scss
@@ -0,0 +1,18 @@
+.homepage {
+margin: auto;
+width: 90%;
+max-width: 80em;
+}
+
+// Make search box wide
+#homepage-searchform {
+width: 100%;
+
+.p-form__group {
+flex-grow: 1;
+}
+
+.p-form__control {
+width: 100%;
+}
+}
diff --git a/lib/canonical/launchpad/images/add-homepage.png b/lib/canonical/launchpad/images/add-homepage.png
new file mode 100644
index 000..0cd9f6f
Binary files /dev/null and b/lib/canonical/launchpad/images/add-homepage.png differ
diff --git a/lib/canonical/launchpad/images/bug-homepage.png b/lib/canonical/launchpad/images/bug-homepage.png
new file mode 100644
index 000..3599ff9
Binary files /dev/null and b/lib/canonical/launchpad/images/bug-homepage.png differ
diff --git a/lib/canonical/launchpad/images/ppa-icon-homepage.png b/lib/canonical/launchpad/images/ppa-icon-homepage.png
new file mode 100644
index 000..f6f529c
Binary files /dev/null and b/lib/canonical/launchpad/images/ppa-icon-homepage.png differ
diff --git a/lib/canonical/launchpad/images/question-homepage.png b/lib/canonical/launchpad/images/question-homepage.png
new file mode 100644
index 000..f43e96f
Binary files /dev/null and b/lib/canonical/launchpad/images/question-homepage.png differ
diff --git a/lib/canonical/launchpad/images/translation-homepage.png b/lib/canonical/launchpad/images/translation-homepage.png
new file mode 100644
index 000..6988a2b
Binary files /dev/null and b/lib/canonical/launchpad/images/translation-homepage.png differ
diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
index c91b890..d6b2760 100644
--- a/lib/lp/app/browser/tales.py
+++ b/lib/lp/app/browser/tales.py
@@ -826,11 +826,10 @@ class ObjectImageDisplayAPI:
 # XXX: this should go away as soon as all image:icon where replaced
 return None
 
-def logo(self):
-"""Return the appropriate  tag for this object's logo.
+def logo_src(self):
+"""Return the appropriate src attribute for this object's logo.
 
-:return: A string, or None if the context object doesn't have
-a logo.
+:return: A string, or None if the context object doesn't have a logo.
 """
 context = self._context
 if not IHasLogo.providedBy(context):
@@ -843,10 +842,18 @@ class ObjectImageDisplayAPI:
 url = context.logo.getURL()
 else:
 url = self.default_logo_resource(context)
-if url is None:
-# We want to indicate that there is no logo for this
-# object.
-return None
+return url
+
+def logo(self):
+"""Return the appropriate  tag for this object's logo.
+
+:return: A string, or None if the context object doesn't have a logo.
+"""
+url = self.logo_src()
+if url is None:
+# We want to indicate that there is no logo for this
+# object.
+return None
 logo = ''
 return logo % url
 
diff --git a/lib/lp/app/templates/root-index.pt b/lib/lp/app/templates/root-index.pt
index ee51d55..16eae94 100644
--- a/lib/lp/app/templates/root-index.pt
+++ b/lib/lp/app/templates/root-index.pt
@@ -6,48 +6,7 @@
   metal:use-macro="view/macro:page/main_only&q

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-change-layout into ~ines-almeida/launchpad:frontpage-revamp-remove-top-project

[Ines Almeida]

The proposal to merge ~ines-almeida/launchpad:frontpage-revamp-change-layout 
into ~ines-almeida/launchpad:frontpage-revamp-remove-top-project has been 
updated.

Status: Needs review => Work in progress

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460819
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-change-layout into 
~ines-almeida/launchpad:frontpage-revamp-remove-top-project.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.

Commit message:
Remove date range from copyright note from lib/lp/code folder

This is an on going process to remove all instances of date ranges in copyright 
notes from the code base to make it easier to maintain. See 
https://code.launchpad.net/\~ines-almeida/launchpad/+git/launchpad/+merge/460944

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460968

This directory (lib/lp/code) is one of the largest sources of copyright range.
This updates 226 out of approx 1700 files.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.
diff --git a/lib/lp/code/adapters/branch.py b/lib/lp/code/adapters/branch.py
index fd62fa5..2bde120 100644
--- a/lib/lp/code/adapters/branch.py
+++ b/lib/lp/code/adapters/branch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Components related to branches."""
diff --git a/lib/lp/code/adapters/gitrepository.py b/lib/lp/code/adapters/gitrepository.py
index 10dd832..e12ea19 100644
--- a/lib/lp/code/adapters/gitrepository.py
+++ b/lib/lp/code/adapters/gitrepository.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Components and adapters related to Git repositories."""
diff --git a/lib/lp/code/adapters/tests/test_branch.py b/lib/lp/code/adapters/tests/test_branch.py
index 3bbd844..8be70be 100644
--- a/lib/lp/code/adapters/tests/test_branch.py
+++ b/lib/lp/code/adapters/tests/test_branch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Functional tests for branch-related components"""
diff --git a/lib/lp/code/browser/branch.py b/lib/lp/code/browser/branch.py
index 467a1bc..c7b8416 100644
--- a/lib/lp/code/browser/branch.py
+++ b/lib/lp/code/browser/branch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Branch views."""
diff --git a/lib/lp/code/browser/branchlisting.py b/lib/lp/code/browser/branchlisting.py
index 721417c..d532057 100644
--- a/lib/lp/code/browser/branchlisting.py
+++ b/lib/lp/code/browser/branchlisting.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Base class view for branch listings."""
diff --git a/lib/lp/code/browser/branchmergeproposal.py b/lib/lp/code/browser/branchmergeproposal.py
index 4d4d0cd..9525cb7 100644
--- a/lib/lp/code/browser/branchmergeproposal.py
+++ b/lib/lp/code/browser/branchmergeproposal.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Views, navigation and actions for BranchMergeProposals."""
diff --git a/lib/lp/code/browser/branchmergeproposallisting.py b/lib/lp/code/browser/branchmergeproposallisting.py
index c51d65f..3fc0ff2 100644
--- a/lib/lp/code/browser/branchmergeproposallisting.py
+++ b/lib/lp/code/browser/branchmergeproposallisting.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Base class view for branch merge proposal listings."""
diff --git a/lib/lp/code/browser/branchsubscription.py b/lib/lp/code/browser/branchsubscription.py
index 5877a14..b1400b1 100644
--- a/lib/lp/code/browser/branchsubscription.py
+++ b/lib/lp/code/browser/branchsubscription.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License vers

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

Diff comments:

> diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
> new file mode 100644
> index 000..286
> --- /dev/null
> +++ b/lp-fetch-service/bundle.yaml
> @@ -0,0 +1,22 @@
> +{%- if stage_name == "production" %}
> +{%-   set devel = False %}
> +{%- elif stage_name == "staging" %}
> +{%-   set devel = False %}
> +{%- else %}
> +{%-   set devel = True %}
> +{%- endif -%}
> +
> +series: jammy
> +applications:
> +{%- if devel or stage_name == "staging" %}

This would throw an error in the stage_name is production because there is no 
application, but I don't think that is an issue. Better keep it safe and ensure 
we don't pollute production.

> +  fetch-service:
> +{#- While the fetch-service charm and snap are not public, we are 
> deploying
> +a locally built charm. The lines below should be replaced with the 
> path
> +to charmhub, channel and revision number once the charm and snap are
> +public. Currently, this expects the charm and snap to be copied to 
> the
> +path where the bundle.yaml is rendered, in the mojo project folder 
> (eg.
> +.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
> +charm: "./fetch-service_ubuntu-22.04-amd64.charm"
> +resources:
> +  snap: "./fetch-service.snap"
> +{%- endif %}


-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/460966
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master.

Commit message:
Add barebones of lp-fetch-service mojo spec

While the charm and snap are not public, they require manually building, and 
deploying from the local charm


Requested reviews:
  Canonical Launchpad Engineering (launchpad)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/460966

This was tested deploying locally.
Will try deploying in staging once this MP is reviewed and approved.
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.
diff --git a/lp-fetch-service/README.md b/lp-fetch-service/README.md
new file mode 100644
index 000..65e09cb
--- /dev/null
+++ b/lp-fetch-service/README.md
@@ -0,0 +1,21 @@
+# Launchpad fetch service
+
+This spec deploys Launchpad's fetch service.
+
+You can run it locally using Juju's LXD support and Mojo. First, configure
+your environment:
+
+export MOJO_ROOT="$HOME/.local/share/mojo"
+export MOJO_PROJECT=mojo-lp-fetch-service
+export MOJO_WORKSPACE=devel
+export MOJO_SERIES=jammy
+export MOJO_SPEC=git+https://git.launchpad.net/~launchpad/launchpad-mojo-specs/+git/private
+export MOJO_STAGE=lp-fetch-service/devel
+
+Then run the spec using Mojo:
+
+mojo project-new -c containerless
+mojo workspace-new
+mojo run
+
+You must have python3-yaml installed.
diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
new file mode 100644
index 000..5e24dfd
--- /dev/null
+++ b/lp-fetch-service/bundle.yaml
@@ -0,0 +1,22 @@
+{%- if stage_name == "production" %}
+{%-   set devel = False %}
+{%- elif stage_name == "staging" %}
+{%-   set devel = False %}
+{%- else %}
+{%-   set devel = True %}
+{%- endif -%}
+
+series: jammy
+applications:
+{%- if not devel or stage_name == "staging" %}
+  fetch-service:
+{#- While the fetch-service charm and snap are not public, we are deploying
+a locally built charm. The lines below should be replaced with the path
+to charmhub, channel and revision number once the charm and snap are
+public. Currently, this expects the charm and snap to be copied to the
+path where the bundle.yaml is rendered, in the mojo project folder (eg.
+.../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
+charm: "./fetch-service_ubuntu-22.04-amd64.charm"
+resources:
+  snap: "./fetch-service.snap"
+{%- endif %}
diff --git a/lp-fetch-service/configs/custom-secgroups-staging.yaml b/lp-fetch-service/configs/custom-secgroups-staging.yaml
new file mode 100644
index 000..78f1546
--- /dev/null
+++ b/lp-fetch-service/configs/custom-secgroups-staging.yaml
@@ -0,0 +1,9 @@
+applications:
+fetch-service:
+type: neutron
+rules:
+- rsync-logs
+rules:
+rsync-logs:
+# Allow launchpad-bastion-ps5 to fetch logs.
+- {"protocol": "tcp", "family": "IPv4", "port": 873, "cidr": "10.131.10.100/32"}
diff --git a/lp-fetch-service/manifest b/lp-fetch-service/manifest
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-perform-autodeploy b/lp-fetch-service/manifest-perform-autodeploy
new file mode 12
index 000..e1c38b1
--- /dev/null
+++ b/lp-fetch-service/manifest-perform-autodeploy
@@ -0,0 +1 @@
+manifests/deploy
\ No newline at end of file
diff --git a/lp-fetch-service/manifest-verify b/lp-fetch-service/manifest-verify
new file mode 12
index 000..6e02de4
--- /dev/null
+++ b/lp-fetch-service/manifest-verify
@@ -0,0 +1 @@
+manifests/verify
\ No newline at end of file
diff --git a/lp-fetch-service/manifests/deploy b/lp-fetch-service/manifests/deploy
new file mode 100644
index 000..8813d9b
--- /dev/null
+++ b/lp-fetch-service/manifests/deploy
@@ -0,0 +1,5 @@
+script config=predeploy
+bundle config=bundle.yaml max-wait=900
+juju-check-wait
+include config=manifests/secgroups
+include config=manifests/verify
diff --git a/lp-fetch-service/manifests/secgroups b/lp-fetch-service/manifests/secgroups
new file mode 100644
index 000..8c438ee
--- /dev/null
+++ b/lp-fetch-service/manifests/secgroups
@@ -0,0 +1 @@
+script config=utils/custom-secgroups.py SKIP_STAGES=devel
diff --git a/lp-fetch-service/manifests/verify b/lp-fetch-service/manifests/verify
new file mode 100644
index 000..f25f902
--- /dev/null
+++ b/lp-fetch-service/manifests/verify
@@ -0,0 +1,4 @@
+juju-check-wait
+# It occasionally takes a little while for all the servers to start
+# accepting connections.
+verify retry=3
diff --git a/lp-fetch-service/predeploy b/lp-fetch-service/predeploy
new file 

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into launchpad-mojo-specs:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service into 
launchpad-mojo-specs:master has been updated.

Description changed to:

This was tested deploying locally. Will try deploying in staging once this MP 
is reviewed and approved.

The general structure was based on similar lp specs.

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/460966
-- 
Your team Launchpad code reviewers is subscribed to branch 
~ines-almeida/launchpad-mojo-specs/+git/private:lp-fetch-service.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-change-layout into ~ines-almeida/launchpad:frontpage-revamp-remove-top-project

[Ines Almeida]

  https://qastaging.launchpad.net/;>sandbox 
> environment.
> -  
> +  
>
>  If you're ready, you can:
> -
> -  
> - -  href="/projects/+new">Register a project
> +
> +
> +  
> +

Done!

> +
> +   href="/projects/+new">Register a project
> +
>
> -  
> - -  href="/people/+newteam">Register a team
> +  
> +
> +
> +   href="/people/+newteam">Register a team
> +
>
> -  
> - -  tal:attributes="href apphomes/bugs">Browse bugs
> +  
> +
> +
> +   href="apphomes/bugs">Browse bugs
> +
>
> -  
> - -  tal:attributes="href apphomes/translations">Help 
> translate
> +  
> + src="/@@/translation-homepage.png">
> +
> +   href="apphomes/translations">Help translate
> +
>
> -  
> - -  tal:attributes="href apphomes/answers">Find answers
> +  
> + src="/@@/question-homepage.png">
> +
> +   href="apphomes/answers">Find answers
> +
>
> -  
> - -  href="/ubuntu/+ppas">Browse Ubuntu PPAs
> +  
> + src="/@@/ppa-icon-homepage.png">
> +
> +   href="/ubuntu/+ppas">Browse Ubuntu PPAs
> +
>
> -  
> - Take the tour
> -   
>  
>
>  
> +  
> +
>  
> -
> -  Featured projects
> -  
> -
> -  
> -
> -  
> -
> -  
> +
> +  
> +
> +  Featured projects
> +
> +
> +  
> +
> +  
> +  
> + href="#" tal:content="structure project/displayname" tal:attributes="href 
> project/fmt:url">
> +
> +
> +
> +  
> +
> +  
>  
>
>  


-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460819
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-change-layout into 
~ines-almeida/launchpad:frontpage-revamp-remove-top-project.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:frontpage-revamp-change-layout into ~ines-almeida/launchpad:frontpage-revamp-remove-top-project

[Ines Almeida]

> These are the same links, right?

Yes, probably a copy-paste error, updated the links now
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460819
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:frontpage-revamp-change-layout into 
~ines-almeida/launchpad:frontpage-revamp-remove-top-project.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

Updated as requested. Unfortunatelly, some of the blog posts initially linked 
are no longer available. But I believe the explanation + knowing this was 
agreed with the legal team should suffice.
-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Commit message changed to:

Remove date range from copyright note in user-facing places

This is no longer necessary and only creates extra work once a year. 
Canonical's legal team agreed with having the start year, but not the range.
Blog post about this topic: https://hynek.me/til/copyright-years/

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Commit message changed to:

Remove date range from copyright note in user-facing places

This is no longer necessary and only creates extra work once a year. 
Canonical's legal team agreed with having the start year, but not the range.

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Description changed to:

Note that in this MP we are not updating the copyright message in every code 
file, only the ones more user facing.

Discussion about this topic: 
https://chat.canonical.com/canonical/pl/e1tmfeisftf9zyhqwqnyzfg8do
Blog post about this topic: https://hynek.me/til/copyright-years/

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Commit message changed to:

Remove date range from copyright note in user-facing places

This is no longer necessary and only creates extra work once a year (see for 
example this blog post https://hynek.me/til/copyright-years/ for the "why"). 
Canonical's legal team agreed with having the start year, but not the range.

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Commit message changed to:

Remove date range from copyright note in user-facing places

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

Ines Almeida has proposed merging 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.

Commit message:
Update Canonical Copyright message from 2004-2023 to 2004-2024 in the frontend

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460944

Note that in this MP we are not updating the copyright message in every code 
file, only the ones more user facing
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.
diff --git a/LICENSE b/LICENSE
index 2f2a36e..30edbc5 100644
--- a/LICENSE
+++ b/LICENSE
@@ -1,4 +1,4 @@
-Launchpad is Copyright 2004-2023 Canonical Ltd.
+Launchpad is Copyright 2004 Canonical Ltd.
 
 Canonical Ltd ("Canonical") distributes the Launchpad source code
 under the GNU Affero General Public License, version 3 ("AGPLv3").
diff --git a/doc/conf.py b/doc/conf.py
index 3f6130a..d2e3538 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -38,7 +38,7 @@ master_doc = "index"
 
 # General information about the project.
 project = "Launchpad"
-copyright = "2004-2023, Canonical Ltd."
+copyright = "2004 Canonical Ltd."
 
 # The version info for the project you're documenting, acts as replacement for
 # |version| and |release|, also used in various other places throughout the
diff --git a/lib/lp/app/stories/basics/copyright.rst b/lib/lp/app/stories/basics/copyright.rst
index 650ed8f..e0952d4 100644
--- a/lib/lp/app/stories/basics/copyright.rst
+++ b/lib/lp/app/stories/basics/copyright.rst
@@ -9,11 +9,11 @@ The tour pages.
 ... find_tag_by_id(browser.contents, "footer-navigation")
 ... )
 ... )
-Next...© 2004-2023 Canonical Ltd...
+Next...© 2004 Canonical Ltd...
 
 The main template.
 
 >>> browser.open("http://launchpad.test;)
 >>> print(extract_text(find_tag_by_id(browser.contents, "footer")))
-© 2004-2023 Canonical Ltd.
+© 2004 Canonical Ltd.
 ...
diff --git a/lib/lp/app/templates/base-layout-macros.pt b/lib/lp/app/templates/base-layout-macros.pt
index e9c2b1d..b812b42 100644
--- a/lib/lp/app/templates/base-layout-macros.pt
+++ b/lib/lp/app/templates/base-layout-macros.pt
@@ -285,7 +285,7 @@
 
 
 
-   2004-2023
+   2004
   http://canonical.com/;>CanonicalLtd.
   
   Terms of use
diff --git a/lib/lp/app/tour/api b/lib/lp/app/tour/api
index fbcf04f..f29f728 100644
--- a/lib/lp/app/tour/api
+++ b/lib/lp/app/tour/api
@@ -129,7 +129,7 @@ We've even done the hard work for you: use our LGPL Python library  Next
 
  
- 2004-2023 Canonical Ltd. https://help.launchpad.net/Legal; accesskey="h" tabindex="6">Terms of Use  https://help.launchpad.net/Feedback; accesskey="i" tabindex="7">Feedback  https://answers.launchpad.net/launchpad-project/+faqs; accesskey="j" tabindex="8">FAQ
+ 2004 Canonical Ltd. https://help.launchpad.net/Legal; accesskey="h" tabindex="6">Terms of Use  https://help.launchpad.net/Feedback; accesskey="i" tabindex="7">Feedback  https://answers.launchpad.net/launchpad-project/+faqs; accesskey="j" tabindex="8">FAQ
 
 
 
diff --git a/lib/lp/app/tour/branch-hosting-tracking b/lib/lp/app/tour/branch-hosting-tracking
index 4c96458..3f31618 100644
--- a/lib/lp/app/tour/branch-hosting-tracking
+++ b/lib/lp/app/tour/branch-hosting-tracking
@@ -125,7 +125,7 @@ All team members can commit to a centrally hosted branch, while anyone can still
 Next
 
 
- 2004-2023 Canonical Ltd. https://help.launchpad.net/Legal; accesskey="h" tabindex="6">Terms of Use  https://help.launchpad.net/Feedback; accesskey="i" tabindex="7">Feedback  https://answers.launchpad.net/launchpad-project/+faqs; accesskey="j" tabindex="8">FAQ
+ 2004 Canonical Ltd. https://help.launchpad.net/Legal; accesskey="h" tabindex="6">Terms of Use  https://help.launchpad.net/Feedback; accesskey="i" tabindex="7">Feedback  https://answers.launchpad.net/launchpad-project/+faqs; accesskey="j" tabindex="8">FAQ
 
 
 
diff --git a/lib/lp/app/tour/bugs b/lib/lp/app/tour/bugs
index 721d914..51f7044 100644
--- a/lib/lp/app/tour/bugs
+++ b/lib/lp/app/tour/bugs
@@ -132,7 +132,7 @@ And to help find low-hanging fruit, theres a Bugs fixed elsewhere&
 Next
 
 
- 2004-2023 Canonical Ltd. https://help.launchpad.net/Legal; 

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Status: Rejected => Superseded

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460927
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~ines-almeida/launchpad:update-copyright-date-across-launchpad into launchpad:master

[Ines Almeida]

The proposal to merge 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master has been updated.

Status: Needs review => Rejected

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/460927
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~ines-almeida/launchpad:update-copyright-date-across-launchpad into 
launchpad:master.


___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp