Answer for an unknown database name with one type
A name nothing covers raised `KeyError` in four places and `AmbiguousDatabaseError` in a fifth, so a caller had to catch both and neither name said what happened. `UnknownDatabaseError` is all of them, exported from `haiku.rag.store` beside the other errors. It subclasses `KeyError`, since selecting by name is a lookup, and prints its message plainly rather than quoted as a missing key. Both CLIs turn it into the same clean exit they already gave the others.
This commit is contained in:
parent
cb9f945c79
commit
b6bc54c69e
13 changed files with 83 additions and 26 deletions
|
|
@ -267,7 +267,10 @@ their documents.
|
|||
none: `search` returns no results, and `ask` and `analyze` run with no evidence
|
||||
from any database.
|
||||
|
||||
On the constructor it means something else. Passing `sources` alongside a
|
||||
A name no client covers raises `UnknownDatabaseError`, a `KeyError`, wherever it
|
||||
is given: at construction, per query, and when placing a citation.
|
||||
|
||||
On the constructor `sources=[]` means something else. Passing `sources` alongside a
|
||||
database path raises `AmbiguousDatabaseError` immediately, since both say which
|
||||
database to open. Passing `sources=[]` alone raises `ValueError` on entering the
|
||||
client: a selection of nothing to search is a legitimate question, a client over
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ from haiku.rag.store.exceptions import ( # noqa: E402
|
|||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
SourceUnavailableError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from haiku.rag.store.models.chunk import SearchType # noqa: E402
|
||||
from haiku.rag.utils import is_up_to_date # noqa: E402
|
||||
|
|
@ -51,6 +52,7 @@ def cli():
|
|||
ConfigMismatchError,
|
||||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
UnknownDatabaseError,
|
||||
SourceUnavailableError,
|
||||
) as e:
|
||||
typer.echo(f"Error: {e}", err=True)
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ from haiku.rag.store.exceptions import (
|
|||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
SourceUnavailableError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from haiku.rag.store.models.chunk import Chunk, SearchResult, SearchType
|
||||
from haiku.rag.store.models.document import Document
|
||||
|
|
@ -208,9 +209,9 @@ class HaikuRAG:
|
|||
|
||||
None only when a client covering a set is given no name, as for evidence
|
||||
recorded before databases could be named. A name this client does not
|
||||
cover raises `KeyError`, decided by `clients_covering` so that one
|
||||
database answers a wrong name the same way a set does: provenance naming
|
||||
another database is wrong rather than absent.
|
||||
cover raises `UnknownDatabaseError`, decided by `clients_covering` so
|
||||
that one database answers a wrong name the same way a set does:
|
||||
provenance naming another database is wrong rather than absent.
|
||||
"""
|
||||
if source is None:
|
||||
return None if self.covers_multiple else self
|
||||
|
|
@ -839,7 +840,7 @@ class HaikuRAG:
|
|||
covered = set(self.source_names)
|
||||
unknown = [name for name in sources if name not in covered]
|
||||
if unknown:
|
||||
raise KeyError(
|
||||
raise UnknownDatabaseError(
|
||||
f"unknown database(s) {', '.join(sorted(set(unknown)))}; this "
|
||||
f"client covers {', '.join(sorted(covered)) or 'a single unnamed database'}"
|
||||
)
|
||||
|
|
@ -864,7 +865,7 @@ class HaikuRAG:
|
|||
if not sources:
|
||||
return []
|
||||
if sources != [self.source]:
|
||||
raise KeyError(
|
||||
raise UnknownDatabaseError(
|
||||
f"unknown database(s) {', '.join(sources) or '(none)'}; this "
|
||||
f"client covers {self.source or 'a single unnamed database'}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -2,7 +2,10 @@ from dataclasses import dataclass
|
|||
from pathlib import Path
|
||||
|
||||
from haiku.rag.config import AppConfig
|
||||
from haiku.rag.store.exceptions import AmbiguousDatabaseError
|
||||
from haiku.rag.store.exceptions import (
|
||||
AmbiguousDatabaseError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from haiku.rag.utils import locate_database
|
||||
|
||||
|
||||
|
|
@ -95,7 +98,7 @@ class DatabaseScope:
|
|||
|
||||
if database_name is not None:
|
||||
if database_name not in declared:
|
||||
raise AmbiguousDatabaseError(
|
||||
raise UnknownDatabaseError(
|
||||
f"unknown database {database_name!r}; lancedb.databases names "
|
||||
f"{', '.join(sorted(declared)) or 'nothing'}"
|
||||
)
|
||||
|
|
@ -129,7 +132,7 @@ class DatabaseScope:
|
|||
by_name = {ref.name: ref for ref in self.databases if ref.name is not None}
|
||||
missing = [name for name in names if name not in by_name]
|
||||
if missing:
|
||||
raise KeyError(
|
||||
raise UnknownDatabaseError(
|
||||
f"unknown database(s) {', '.join(sorted(missing))}; "
|
||||
f"configured: {', '.join(sorted(by_name))}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ from haiku.rag.store.exceptions import (
|
|||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
SourceUnavailableError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from haiku.rag.store.repositories.chunk import ChunkRepository
|
||||
from haiku.rag.store.repositories.document import DocumentRepository
|
||||
|
|
@ -288,7 +289,7 @@ class FederatedSession:
|
|||
"""
|
||||
unknown = [name for name in names if name not in self._refs]
|
||||
if unknown:
|
||||
raise KeyError(
|
||||
raise UnknownDatabaseError(
|
||||
f"unknown database(s) {', '.join(sorted(unknown))}; configured: "
|
||||
f"{', '.join(sorted(self._refs))}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ from haiku.rag.store.exceptions import ( # noqa: E402
|
|||
AmbiguousDatabaseError,
|
||||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
|
|
@ -71,7 +72,12 @@ def cli() -> None:
|
|||
"""Entry point that translates store-state errors into a clean exit."""
|
||||
try:
|
||||
_cli()
|
||||
except (AmbiguousDatabaseError, MigrationRequiredError, ReadOnlyError) as e:
|
||||
except (
|
||||
AmbiguousDatabaseError,
|
||||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
UnknownDatabaseError,
|
||||
) as e:
|
||||
typer.echo(f"Error: {e}", err=True)
|
||||
sys.exit(1)
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ from .exceptions import (
|
|||
MigrationRequiredError,
|
||||
ReadOnlyError,
|
||||
SourceUnavailableError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from .models import Chunk, Document
|
||||
|
||||
|
|
@ -17,4 +18,5 @@ __all__ = [
|
|||
"AmbiguousDatabaseError",
|
||||
"ConfigMismatchError",
|
||||
"SourceUnavailableError",
|
||||
"UnknownDatabaseError",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -24,6 +24,19 @@ class AmbiguousDatabaseError(Exception):
|
|||
"""
|
||||
|
||||
|
||||
class UnknownDatabaseError(KeyError):
|
||||
"""A name that does not belong to the databases in question.
|
||||
|
||||
A `KeyError`, since selecting by name is a lookup, and one type wherever a
|
||||
name is checked: at construction, per query, and when placing evidence.
|
||||
"""
|
||||
|
||||
def __str__(self) -> str:
|
||||
# KeyError quotes its argument, which reads as a missing dict key rather
|
||||
# than the sentence this carries.
|
||||
return str(self.args[0]) if self.args else ""
|
||||
|
||||
|
||||
class AmbiguousCitationError(Exception):
|
||||
"""A cited chunk id names a chunk in more than one database.
|
||||
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from haiku.rag.client import HaikuRAG
|
|||
from haiku.rag.client.scope import DatabaseScope
|
||||
from haiku.rag.client.session import FederatedSession
|
||||
from haiku.rag.sandbox import AnalysisContext, Sandbox
|
||||
from haiku.rag.store.exceptions import UnknownDatabaseError
|
||||
from haiku.rag.store.models import SearchResult
|
||||
from tests.multi_db.helpers import (
|
||||
_config,
|
||||
|
|
@ -212,7 +213,7 @@ class TestNamingDatabasesBeforeTheModelRuns:
|
|||
await _seed(config, "beta", ["beta document about cats"])
|
||||
|
||||
async with HaikuRAG(config=config) as rag:
|
||||
with pytest.raises(KeyError, match="typo"):
|
||||
with pytest.raises(UnknownDatabaseError, match="typo"):
|
||||
await rag.ask("what about cats?", sources=["typo"])
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -221,7 +222,7 @@ class TestNamingDatabasesBeforeTheModelRuns:
|
|||
await _seed(config, "alpha", ["alpha document about cats"])
|
||||
|
||||
async with HaikuRAG(config=config) as rag:
|
||||
with pytest.raises(KeyError, match="typo"):
|
||||
with pytest.raises(UnknownDatabaseError, match="typo"):
|
||||
await rag.analyze("how many?", sources=["typo"])
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -238,7 +239,7 @@ class TestNamingDatabasesBeforeTheModelRuns:
|
|||
rag._require_known_sources(None)
|
||||
rag._require_known_sources(["alpha"])
|
||||
rag._require_known_sources([])
|
||||
with pytest.raises(KeyError, match="typo"):
|
||||
with pytest.raises(UnknownDatabaseError, match="typo"):
|
||||
rag._require_known_sources(["alpha", "typo"])
|
||||
|
||||
assert rag._session._sessions == {}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,10 @@ from pydantic import ValidationError
|
|||
from haiku.rag.client import HaikuRAG
|
||||
from haiku.rag.client.scope import DatabaseScope
|
||||
from haiku.rag.config.models import AppConfig, LanceDBConfig
|
||||
from haiku.rag.store.exceptions import AmbiguousDatabaseError
|
||||
from haiku.rag.store.exceptions import (
|
||||
AmbiguousDatabaseError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from haiku.rag.utils import locate_database
|
||||
from tests.multi_db.helpers import (
|
||||
_config,
|
||||
|
|
@ -147,7 +150,7 @@ class TestSelection:
|
|||
async def test_unknown_source_at_construction_is_rejected(self, tmp_path):
|
||||
config = _config(tmp_path, ["alpha", "beta"])
|
||||
|
||||
with pytest.raises(KeyError, match="nope"):
|
||||
with pytest.raises(UnknownDatabaseError, match="nope"):
|
||||
async with HaikuRAG(config=config, sources=["nope"]):
|
||||
pass
|
||||
|
||||
|
|
@ -158,7 +161,7 @@ class TestSelection:
|
|||
await _seed(config, "beta", ["beta document about cats"])
|
||||
|
||||
async with HaikuRAG(config=config) as rag:
|
||||
with pytest.raises(KeyError, match="nope"):
|
||||
with pytest.raises(UnknownDatabaseError, match="nope"):
|
||||
await rag.search("cats", search_type="fts", sources=["nope"])
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -238,6 +241,23 @@ class TestPlacingADatabase:
|
|||
async with HaikuRAG(temp_db_path, create=True) as rag:
|
||||
assert await rag.reader_for(None) is rag
|
||||
|
||||
def test_one_type_answers_for_a_name_nothing_covers(self, tmp_path):
|
||||
"""Selecting by name is a lookup wherever it happens, so it fails the
|
||||
same way at construction, per query, and when placing evidence."""
|
||||
config = _config(tmp_path, ["alpha", "beta"])
|
||||
|
||||
with pytest.raises(UnknownDatabaseError):
|
||||
DatabaseScope.resolve(config, database_name="typo")
|
||||
with pytest.raises(UnknownDatabaseError):
|
||||
DatabaseScope.resolve(config).select(["typo"])
|
||||
|
||||
# A KeyError, so a caller treating selection as a lookup still catches it.
|
||||
assert issubclass(UnknownDatabaseError, KeyError)
|
||||
# ...but the message reads as a sentence rather than a missing key.
|
||||
assert str(UnknownDatabaseError("unknown database 'typo'")) == (
|
||||
"unknown database 'typo'"
|
||||
)
|
||||
|
||||
def test_a_path_and_sources_cannot_both_choose(self, tmp_path):
|
||||
"""`sources` used to be ignored beside a path, so selecting a database
|
||||
that is not the one at the path opened the path anyway."""
|
||||
|
|
@ -257,14 +277,14 @@ class TestPlacingADatabase:
|
|||
|
||||
async with HaikuRAG(config=config, sources=["alpha"]) as alpha:
|
||||
assert await alpha.reader_for("alpha") is alpha
|
||||
with pytest.raises(KeyError, match="beta"):
|
||||
with pytest.raises(UnknownDatabaseError, match="beta"):
|
||||
await alpha.reader_for("beta")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_unnamed_database_refuses_any_name(self, temp_db_path):
|
||||
"""Nothing names it, so no name can be the one it covers."""
|
||||
async with HaikuRAG(temp_db_path, create=True) as rag:
|
||||
with pytest.raises(KeyError, match="single unnamed database"):
|
||||
with pytest.raises(UnknownDatabaseError, match="single unnamed database"):
|
||||
await rag.reader_for("anything")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from haiku.rag.config import get_config
|
|||
from haiku.rag.store.exceptions import (
|
||||
ConfigMismatchError,
|
||||
SourceUnavailableError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from haiku.rag.store.models import Chunk, DocumentItem
|
||||
from tests.multi_db.helpers import (
|
||||
|
|
@ -54,7 +55,7 @@ class TestFederatedSearch:
|
|||
await _seed(config, "alpha", ["alpha document about cats"])
|
||||
|
||||
async with HaikuRAG(config=config) as rag:
|
||||
with pytest.raises(KeyError, match="nope"):
|
||||
with pytest.raises(UnknownDatabaseError, match="nope"):
|
||||
await rag.search("cats", search_type="fts", sources=["nope"])
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ from haiku.rag.config.models import AppConfig, LanceDBConfig, StorageConfig
|
|||
from haiku.rag.store.exceptions import (
|
||||
AmbiguousDatabaseError,
|
||||
MigrationRequiredError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
from tests.conftest import for_path
|
||||
|
||||
|
|
@ -207,14 +208,14 @@ class TestSelectingADatabaseByName:
|
|||
self._install(monkeypatch, alpha="/data/a.lancedb", beta="/data/b.lancedb")
|
||||
monkeypatch.setattr("haiku.rag.cli._db_name", "gamma")
|
||||
|
||||
with pytest.raises(AmbiguousDatabaseError, match="alpha, beta"):
|
||||
with pytest.raises(UnknownDatabaseError, match="alpha, beta"):
|
||||
resolve_scope(None)
|
||||
|
||||
def test_an_unknown_name_does_not_leak_locations(self, monkeypatch):
|
||||
self._install(monkeypatch, papers="s3://bucket/prefix/papers.lancedb")
|
||||
monkeypatch.setattr("haiku.rag.cli._db_name", "gamma")
|
||||
|
||||
with pytest.raises(AmbiguousDatabaseError) as raised:
|
||||
with pytest.raises(UnknownDatabaseError) as raised:
|
||||
resolve_scope(None)
|
||||
|
||||
assert "bucket" not in str(raised.value)
|
||||
|
|
@ -223,7 +224,7 @@ class TestSelectingADatabaseByName:
|
|||
self._install(monkeypatch)
|
||||
monkeypatch.setattr("haiku.rag.cli._db_name", "papers")
|
||||
|
||||
with pytest.raises(AmbiguousDatabaseError, match="nothing"):
|
||||
with pytest.raises(UnknownDatabaseError, match="nothing"):
|
||||
resolve_scope(None)
|
||||
|
||||
def test_the_callback_selects_before_a_command_runs(self, tmp_path, monkeypatch):
|
||||
|
|
@ -240,7 +241,7 @@ class TestSelectingADatabaseByName:
|
|||
)
|
||||
|
||||
assert result.exit_code != 0
|
||||
assert isinstance(result.exception, AmbiguousDatabaseError)
|
||||
assert isinstance(result.exception, UnknownDatabaseError)
|
||||
assert "nope" in str(result.exception)
|
||||
|
||||
def test_a_selection_does_not_outlive_its_invocation(self, tmp_path, monkeypatch):
|
||||
|
|
|
|||
|
|
@ -5,7 +5,10 @@ import pytest
|
|||
from haiku.rag.client.scope import DatabaseRef, DatabaseScope
|
||||
from haiku.rag.client.session import default_db_path
|
||||
from haiku.rag.config.models import AppConfig, LanceDBConfig, StorageConfig
|
||||
from haiku.rag.store.exceptions import AmbiguousDatabaseError
|
||||
from haiku.rag.store.exceptions import (
|
||||
AmbiguousDatabaseError,
|
||||
UnknownDatabaseError,
|
||||
)
|
||||
|
||||
|
||||
def _config(**kwargs) -> AppConfig:
|
||||
|
|
@ -45,7 +48,7 @@ class TestResolution:
|
|||
def test_an_unknown_name_is_refused(self):
|
||||
config = _config(databases={"alpha": "/data/alpha.lancedb"})
|
||||
|
||||
with pytest.raises(AmbiguousDatabaseError, match="unknown database 'nope'"):
|
||||
with pytest.raises(UnknownDatabaseError, match="unknown database 'nope'"):
|
||||
DatabaseScope.resolve(config, database_name="nope")
|
||||
|
||||
def test_no_selector_covers_the_configured_set_in_order(self):
|
||||
|
|
|
|||
Loading…
Reference in a new issue