feat(quantum): RelayPool — multi-relay fanout + failover for the artifact store#26
Merged
Conversation
…fact store
A single relay was a single point of failure for P2P artifact sharing. RelayPool
lets KNITWEB_RELAY be a comma-separated list (or iterable): pushes fan out to
every relay best-effort (success if >=1 accepts, warns on partial outage), and
pulls fail over to each relay in order until one returns the artifact.
- relaypool.py — RelayPool{push,pull,primary} with an injectable HTTP transport
(put_fn/get_fn), fully testable offline; normalizes str/list/None, strips
trailing slashes, dedups.
- store.py — Store holds a RelayPool (from relay arg or KNITWEB_RELAY);
`store.relay` kept as backward-compatible primary accessor.
Backward compatible: a single relay URL behaves exactly as before. Relates to #275.
Tests: tests/quantum/test_relaypool.py (11) — normalization, fanout, partial-outage
best-effort, pull failover, KeyError-when-none, Store integration incl. primary-down
failover. Full quantum suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideIntroduce a RelayPool abstraction that supports multi-relay fanout for pushes and ordered failover for pulls, and integrate it into the Store while keeping the public API backward compatible and fully testable offline. Sequence diagram for Store.get failover via RelayPool.pullsequenceDiagram
actor User
participant Store
participant RelayPool
participant Relay1
participant Relay2
User->>Store: get(cid)
Store->>Store: _path(cid).exists()
alt [local missing]
Store->>RelayPool: pull(collection, cid)
alt [Relay1 has artifact]
RelayPool->>Relay1: _get(url)
Relay1-->>RelayPool: bytes
RelayPool-->>Store: bytes
Store->>Store: path.write_bytes(bytes)
else [Relay1 fails, Relay2 has artifact]
RelayPool->>Relay1: _get(url)
Relay1-->>RelayPool: Exception
RelayPool->>Relay2: _get(url)
Relay2-->>RelayPool: bytes
RelayPool-->>Store: bytes
Store->>Store: path.write_bytes(bytes)
else [no relay has artifact]
RelayPool-->>Store: KeyError
Store-->>User: KeyError
end
else [local exists]
Store->>Store: load artifact
Store-->>User: artifact
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider restoring and expanding the type hint on
Store.__init__(relay=...)to make it clear it accepts a string, comma-separated string, or iterable of URLs (e.g.str | Iterable[str] | None), which will help static analysis and readability. - In
RelayPool.pushandRelayPool.pull, you currently catchExceptionbroadly; if practical, narrowing this to the specific network/transport errors you expect would make failure modes clearer and avoid masking unexpected exceptions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider restoring and expanding the type hint on `Store.__init__(relay=...)` to make it clear it accepts a string, comma-separated string, or iterable of URLs (e.g. `str | Iterable[str] | None`), which will help static analysis and readability.
- In `RelayPool.push` and `RelayPool.pull`, you currently catch `Exception` broadly; if practical, narrowing this to the specific network/transport errors you expect would make failure modes clearer and avoid masking unexpected exceptions.
## Individual Comments
### Comment 1
<location path="src/knitweb_lens/quantum/store.py" line_range="81" />
<code_context>
"""
- def __init__(self, root: str | Path | None = None, relay: str | None = None):
+ def __init__(self, root: str | Path | None = None, relay=None):
self.root = Path(root) if root else _DEFAULT_DIR
self.root.mkdir(parents=True, exist_ok=True)
</code_context>
<issue_to_address>
**suggestion:** Keep type information for `relay` consistent with supported inputs.
`relay` is now untyped, but RelayPool supports strings, comma‑separated strings, and iterables. Please restore a precise annotation (e.g. `relay: str | Iterable[str] | None = None` or a typed alias) so valid input shapes are clear and static analysis can catch mismatches.
Suggested implementation:
```python
from .relaypool import RelayPool
from collections.abc import Iterable
```
```python
def __init__(self, root: str | Path | None = None, relay: str | Iterable[str] | None = None):
```
</issue_to_address>
### Comment 2
<location path="src/knitweb_lens/quantum/relaypool.py" line_range="109-110" />
<code_context>
+ return self._get(url)
+ except Exception as e: # noqa: BLE001 - try the next relay
+ errors.append(f"{base}: {e}")
+ raise KeyError(f"{cid} not found on any relay ({len(self.relays)} tried): "
+ + "; ".join(errors) if errors else f"{cid}: no relays configured")
+
+ def __repr__(self) -> str:
</code_context>
<issue_to_address>
**suggestion:** Clarify the `KeyError` message construction to avoid precedence surprises.
The combined use of `+` with a conditional expression makes it unclear which parts of the string are conditionally included, and is easy to misinterpret or break if modified. Please build the error message in a separate variable using an explicit `if errors: ... else: ...` before raising, so the logic is clearer and safer to maintain.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| """ | ||
|
|
||
| def __init__(self, root: str | Path | None = None, relay: str | None = None): | ||
| def __init__(self, root: str | Path | None = None, relay=None): |
There was a problem hiding this comment.
suggestion: Keep type information for relay consistent with supported inputs.
relay is now untyped, but RelayPool supports strings, comma‑separated strings, and iterables. Please restore a precise annotation (e.g. relay: str | Iterable[str] | None = None or a typed alias) so valid input shapes are clear and static analysis can catch mismatches.
Suggested implementation:
from .relaypool import RelayPool
from collections.abc import Iterable def __init__(self, root: str | Path | None = None, relay: str | Iterable[str] | None = None):
Comment on lines
+109
to
+110
| raise KeyError(f"{cid} not found on any relay ({len(self.relays)} tried): " | ||
| + "; ".join(errors) if errors else f"{cid}: no relays configured") |
There was a problem hiding this comment.
suggestion: Clarify the KeyError message construction to avoid precedence surprises.
The combined use of + with a conditional expression makes it unclear which parts of the string are conditionally included, and is easy to misinterpret or break if modified. Please build the error message in a separate variable using an explicit if errors: ... else: ... before raising, so the logic is clearer and safer to maintain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the single-relay SPOF:
KNITWEB_RELAYcan now be a comma-separated list. Pushes fan out to all relays (best-effort, warns on partial outage); pulls fail over in order. Backward compatible; injectable transport, 11 offline tests. Relates to #275.🤖 Generated with Claude Code
Summary by Sourcery
Introduce a RelayPool abstraction for multi-relay fanout and failover in the quantum artifact store and integrate it into the Store API.
New Features:
Tests: