Colin Watson has proposed merging
~cjwatson/launchpad:fix-git-builder-constraints into launchpad:master.
Commit message:
Avoid unnecessary queries in BuilderResource vocabulary
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/433784
Using a `SimpleVocabulary` means we have to find the available terms before
instantiating the vocabulary. Reimplement this a bit more manually to avoid
that problem.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad:fix-git-builder-constraints into launchpad:master.
diff --git a/lib/lp/buildmaster/tests/test_vocabularies.py b/lib/lp/buildmaster/tests/test_vocabularies.py
new file mode 100644
index 0000000..f644a43
--- /dev/null
+++ b/lib/lp/buildmaster/tests/test_vocabularies.py
@@ -0,0 +1,99 @@
+# Copyright 2022 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from zope.schema.interfaces import ITerm, ITokenizedTerm, IVocabularyTokenized
+from zope.schema.vocabulary import getVocabularyRegistry
+
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import ZopelessDatabaseLayer
+
+
+class TestBuilderResourceVocabulary(TestCaseWithFactory):
+
+ layer = ZopelessDatabaseLayer
+
+ def test_provides_interface(self):
+ self.factory.makeBuilder()
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertProvides(vocab, IVocabularyTokenized)
+
+ def test___contains__(self):
+ self.factory.makeBuilder(open_resources=["large"])
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertIn("large", vocab)
+ self.assertNotIn("small", vocab)
+
+ def test___len__(self):
+ self.factory.makeBuilder(open_resources=["large", "larger"])
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertEqual(2, len(vocab))
+
+ def test_getTerm(self):
+ self.factory.makeBuilder(open_resources=["large"])
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ term = vocab.getTerm("large")
+ self.assertProvides(term, ITerm)
+ self.assertEqual("large", term.value)
+ self.assertRaises(LookupError, vocab.getTerm, "small")
+
+ def test_getTermByToken(self):
+ self.factory.makeBuilder(open_resources=["large"])
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ term = vocab.getTermByToken("large")
+ self.assertProvides(term, ITokenizedTerm)
+ self.assertEqual("large", term.value)
+ self.assertEqual("large", term.token)
+ self.assertRaises(LookupError, vocab.getTerm, "small")
+
+ def test_no_resources(self):
+ self.factory.makeBuilder()
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertEqual([], list(vocab))
+
+ def test_current_resources(self):
+ for open_resources, restricted_resources in (
+ (None, None),
+ (["large"], None),
+ (["large", "larger"], None),
+ (None, ["gpu"]),
+ (["large"], ["gpu"]),
+ ):
+ self.factory.makeBuilder(
+ open_resources=open_resources,
+ restricted_resources=restricted_resources,
+ )
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertEqual(
+ ["gpu", "large", "larger"], [term.value for term in vocab]
+ )
+ self.assertEqual(
+ ["gpu", "large", "larger"], [term.token for term in vocab]
+ )
+
+ def test_merges_constraints_from_context(self):
+ self.factory.makeBuilder(open_resources=["large"])
+ repository = self.factory.makeGitRepository(
+ builder_constraints=["really-large"]
+ )
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertEqual(
+ ["large", "really-large"], [term.value for term in vocab]
+ )
+ self.assertEqual(
+ ["large", "really-large"], [term.token for term in vocab]
+ )
+
+ def test_skips_invisible_builders(self):
+ self.factory.makeBuilder(open_resources=["large"])
+ self.factory.makeBuilder(active=False, open_resources=["old"])
+ repository = self.factory.makeGitRepository()
+ vocab = getVocabularyRegistry().get(repository, "BuilderResource")
+ self.assertEqual(["large"], [term.value for term in vocab])
+ self.assertEqual(["large"], [term.token for term in vocab])
diff --git a/lib/lp/buildmaster/vocabularies.py b/lib/lp/buildmaster/vocabularies.py
index e7f8dbe..154e445 100644
--- a/lib/lp/buildmaster/vocabularies.py
+++ b/lib/lp/buildmaster/vocabularies.py
@@ -4,17 +4,20 @@
"""Soyuz vocabularies."""
__all__ = [
- "builder_resource_vocabulary_factory",
+ "BuilderResourceVocabulary",
"ProcessorVocabulary",
]
from storm.expr import Alias, Cast, Coalesce, Func
-from zope.schema.vocabulary import SimpleTerm, SimpleVocabulary
+from zope.interface import implementer
+from zope.schema.interfaces import IVocabularyTokenized
+from zope.schema.vocabulary import SimpleTerm
from lp.buildmaster.model.builder import Builder
from lp.buildmaster.model.processor import Processor
from lp.services.database.interfaces import IStore
from lp.services.database.stormexpr import Concatenate
+from lp.services.propertycache import cachedproperty
from lp.services.webapp.vocabulary import NamedSQLObjectVocabulary
@@ -25,33 +28,70 @@ class ProcessorVocabulary(NamedSQLObjectVocabulary):
_orderBy = "name"
-def builder_resource_vocabulary_factory(context):
- """Return a vocabulary of all currently-defined builder resources.
+@implementer(IVocabularyTokenized)
+class BuilderResourceVocabulary:
+ """A vocabulary of all currently-defined builder resources.
The context is anything with a `builder_constraints` attribute; the
current constraints are merged into the set of available builder
resources, to avoid problems if some unknown resources are already set
as constraints.
"""
- resources = set(
- IStore(Builder)
- .find(
- Alias(
- Func(
- "jsonb_array_elements_text",
- Concatenate(
- Coalesce(Builder.open_resources, Cast("[]", "jsonb")),
- Coalesce(
- Builder.restricted_resources, Cast("[]", "jsonb")
+
+ def __init__(self, context):
+ self.context = context
+
+ @cachedproperty
+ def _resources(self):
+ builder_resources = set(
+ IStore(Builder)
+ .find(
+ Alias(
+ Func(
+ "jsonb_array_elements_text",
+ Concatenate(
+ Coalesce(
+ Builder.open_resources, Cast("[]", "jsonb")
+ ),
+ Coalesce(
+ Builder.restricted_resources,
+ Cast("[]", "jsonb"),
+ ),
),
),
+ "resource",
),
- "resource",
- ),
- Builder.active,
+ Builder.active,
+ )
+ .config(distinct=True)
)
- .config(distinct=True)
- ).union(context.builder_constraints or set())
- return SimpleVocabulary(
- [SimpleTerm(resource) for resource in sorted(resources)]
- )
+ return sorted(
+ builder_resources.union(self.context.builder_constraints or set())
+ )
+
+ def __contains__(self, value):
+ """See `zope.schema.interfaces.ISource`."""
+ return value in self._resources
+
+ def __iter__(self):
+ """See `zope.schema.interfaces.IIterableVocabulary`."""
+ for resource in self._resources:
+ yield SimpleTerm(resource)
+
+ def __len__(self):
+ """See `zope.schema.interfaces.IIterableVocabulary`."""
+ return len(self._resources)
+
+ def getTerm(self, value):
+ """See `zope.schema.interfaces.IBaseVocabulary`."""
+ if value in self._resources:
+ return SimpleTerm(value)
+ else:
+ raise LookupError(value)
+
+ def getTermByToken(self, token):
+ """See `zope.schema.interfaces.IVocabularyTokenized`."""
+ if token in self._resources:
+ return SimpleTerm(token)
+ else:
+ raise LookupError(token)
diff --git a/lib/lp/buildmaster/vocabularies.zcml b/lib/lp/buildmaster/vocabularies.zcml
index 449059f..ba5f97f 100644
--- a/lib/lp/buildmaster/vocabularies.zcml
+++ b/lib/lp/buildmaster/vocabularies.zcml
@@ -18,10 +18,14 @@
<securedutility
name="BuilderResource"
- component="lp.buildmaster.vocabularies.builder_resource_vocabulary_factory"
+ component="lp.buildmaster.vocabularies.BuilderResourceVocabulary"
provides="zope.schema.interfaces.IVocabularyFactory"
>
<allow interface="zope.schema.interfaces.IVocabularyFactory"/>
</securedutility>
+ <class class="lp.buildmaster.vocabularies.BuilderResourceVocabulary">
+ <allow interface="lp.services.webapp.vocabulary.IVocabularyTokenized"/>
+ </class>
+
</configure>
_______________________________________________
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
