Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions libby/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ def _chunked(items: Sequence[str], size: int) -> Iterator[Sequence[str]]:
class KeyListing:
"""One peer's answer to a ``keys.list`` request.

``writeonly`` names those of ``names`` that have no value to read, so a
caller polling the peer can skip them. It is bare keyword names, matching
``matches`` on the wire rather than the qualified ``names`` above.

``services`` lists the non-keyword keys the peer answers, such as
``keys.read``. It is empty for a peer on a libby old enough not to report
the field, which is how a caller decides whether bulk reads are available:
Expand All @@ -71,6 +75,7 @@ class KeyListing:
"""

names: Tuple[str, ...]
writeonly: Tuple[str, ...]
services: Tuple[str, ...]


Expand Down Expand Up @@ -157,6 +162,7 @@ def listing(self, pattern: str, *, timeout_s: float = DEFAULT_TIMEOUT_S) -> KeyL
return KeyListing(
names=tuple(f"{group}.{daemon}.{match}"
for match in response.get("matches", [])),
writeonly=tuple(response.get("writeonly", [])),
services=tuple(response.get("services", [])),
)

Expand Down
14 changes: 9 additions & 5 deletions libby/keygrabber/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,18 @@ def backoff_interval_s(self) -> float:
def resolve(self, client: Client) -> Tuple[str, ...]:
"""Ask the peer what it serves and select the configured keywords.

One ``keys.list`` covers both: the peer's keyword names, and whether it
serves ``keys.read``. Selection is then local, so a collection with
several patterns still costs one request.
One ``keys.list`` covers all of it: the peer's keyword names, which of
them cannot be read, and whether it serves ``keys.read``. Selection is
then local, so a collection with several patterns still costs one
request.
"""
listing = client.listing(f"{self.config.peer}.%",
timeout_s=self.config.timeout_s)
available = [name.rsplit(".", 1)[-1] for name in listing.names]
self._names = select_keywords(self.config, available)
# A trigger answers a show with `false`, and a setter-only keyword with
# an error, so neither is worth asking for on every tick
readable = [name.rsplit(".", 1)[-1] for name in listing.names
if name.rsplit(".", 1)[-1] not in listing.writeonly]
self._names = select_keywords(self.config, readable)
self._bulk_read = BULK_READ_SERVICE in listing.services
self._resolved_at = self._clock()
return self._names
Expand Down
8 changes: 7 additions & 1 deletion libby/libby.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,15 @@ def _keys_list(self, payload: dict, _ctx: dict) -> dict:
pattern = payload.get("pattern", "%")
if not isinstance(pattern, str):
return {"ok": False, "error": "pattern must be a string"}
matches = match_pattern(pattern, self._keywords)
return {
"ok": True,
"matches": match_pattern(pattern, self._keywords),
"matches": matches,
# Which of those have no value to read, so a caller polling a peer
# does not ask for them. A trigger answers a show with `false`,
# which would otherwise be archived as data.
"writeonly": [name for name in matches
if self._keywords[name].writeonly],
"services": self._served_services(),
"is_daemon": self.is_daemon,
}
Expand Down
15 changes: 15 additions & 0 deletions tests/test_client_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ def on_start(self, libby) -> None:
units="mm",
)
self.keyword_registry.bool("isreferenced", getter=lambda: True)
# Setter-only, like a gauge's unit selector: listed, but showing it
# yields an error rather than a value
self.keyword_registry.int("mode", setter=lambda v: state.update(mode=v))


class _LifecycleDaemon(LibbyDaemon):
Expand Down Expand Up @@ -193,6 +196,18 @@ def test_listing_advertises_bulk_read(self):
self.assertIn("keys.read", listing.services)
self.assertIn(self._name("positionvalue"), listing.names)

def test_listing_names_the_keywords_that_cannot_be_read(self):
"""A setter-only keyword is listed, and flagged as unreadable.

It stays in names so `libby list` still shows it; a caller polling
the peer uses writeonly to skip it rather than reading an error
every tick.
"""
listing = self.client.listing(self._name("%"), timeout_s=RPC_TIMEOUT_S)
self.assertIn(self._name("mode"), listing.names)
self.assertEqual(listing.writeonly, ("mode",))
self.assertNotIn("positionvalue", listing.writeonly)

def test_describe_reports_type_and_units(self):
"""Read one keyword's metadata."""
meta = self.client.describe(self._name("positionvalue"),
Expand Down
26 changes: 26 additions & 0 deletions tests/test_keygrabber_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ def __init__(
self,
names: Sequence[str] = ("positionvalue", "isconnected"),
services: Sequence[str] = ("keys.read",),
writeonly: Sequence[str] = (),
values: Optional[Dict[str, Dict[str, Any]]] = None,
timeouts_after: Optional[int] = None,
) -> None:
self.names = list(names)
self.services = list(services)
self.writeonly = list(writeonly)
self.values = values or {}
self.timeouts_after = timeouts_after
self.read_calls: List[Tuple[str, ...]] = []
Expand All @@ -61,6 +63,7 @@ def listing(self, pattern: str, *, timeout_s: float = 3.0) -> KeyListing:
prefix = pattern.rsplit(".", 1)[0]
return KeyListing(
names=tuple(f"{prefix}.{name}" for name in self.names),
writeonly=tuple(self.writeonly),
services=tuple(self.services),
)

Expand Down Expand Up @@ -209,6 +212,29 @@ def test_fallback_abandons_the_tick_after_a_timeout(self):
self.assertEqual(result.read_errors, 4 - 1)
self.assertEqual(len(result.samples), 1)

def test_write_only_keywords_are_never_read(self):
"""A trigger answers a show with `false`, which is not a measurement.

Archiving it would record `shutdown` as a datapoint on every peer at
every interval, forever.
"""
client = _FakeClient(names=("pressure", "shutdown", "units_code"),
writeonly=("shutdown", "units_code"),
values={"pressure": {"ok": True, "value": 1.0}})
collection = Collection(_collection_config())

self.assertEqual(collection.resolve(client), ("pressure",))
result = collection.tick(client, TIMESTAMP)
self.assertEqual(client.read_calls, [("hsfei.adc.pressure",)])
self.assertEqual(result.read_errors, 0)
self.assertEqual([s.keyword for s in result.samples], ["pressure"])

def test_a_peer_reporting_none_is_read_as_before(self):
"""An older libby omits the field, so nothing is filtered."""
client = _FakeClient(names=("pressure", "shutdown"), writeonly=())
self.assertEqual(Collection(_collection_config()).resolve(client),
("pressure", "shutdown"))


if __name__ == "__main__":
unittest.main()
Loading