mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(cli): clean error on invalid share --expires-at and drop dead schemas
Address review findings on `bm cloud share`: - `create` re-wrapped the `typer.Exit(1)` from `_parse_expires_at()` as a spurious "Unexpected error: 1" trailing line because the broad `except Exception` caught it (typer.Exit subclasses Exception). Parse and validate --expires-at before entering the try block so the error surfaces as a single clean message, and add the `except typer.Exit: raise` guard for parity with `update`/`revoke`. - Strengthen test_create_share_invalid_expires_at to assert "Unexpected error" is absent, which was masking the bug. - Drop unused PublicShareResponse/PublicShareListResponse schemas; responses are parsed as raw dicts, so the schemas were dead code. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Drew Cain <groksrc@gmail.com>
This commit is contained in:
@@ -53,30 +53,3 @@ class BucketSnapshotRestoreResponse(BaseModel):
|
||||
restored: list[str]
|
||||
snapshot_version: str
|
||||
snapshot_id: UUID
|
||||
|
||||
|
||||
class PublicShareResponse(BaseModel):
|
||||
"""Response model for a public share link.
|
||||
|
||||
Mirrors PublicShareResponse in basic-memory-cloud
|
||||
(apps/cloud/.../schemas/public_share_schemas.py).
|
||||
"""
|
||||
|
||||
id: UUID
|
||||
token: str
|
||||
project_name: str
|
||||
note_permalink: str
|
||||
note_external_id: str
|
||||
enabled: bool
|
||||
expires_at: datetime | None
|
||||
share_url: str
|
||||
view_count: int
|
||||
last_viewed_at: datetime | None
|
||||
created_at: datetime
|
||||
|
||||
|
||||
class PublicShareListResponse(BaseModel):
|
||||
"""Response from listing public shares."""
|
||||
|
||||
shares: list[PublicShareResponse]
|
||||
total: int
|
||||
|
||||
@@ -96,19 +96,22 @@ def create(
|
||||
bm cloud share create my-project notes/my-idea --expires-at 2025-12-31
|
||||
"""
|
||||
|
||||
# Validate --expires-at before any async/API work so a parse error surfaces
|
||||
# a single clean message and exits, rather than being re-wrapped by the broad
|
||||
# handler below as "Unexpected error: 1" (typer.Exit subclasses Exception).
|
||||
payload: dict = {
|
||||
"project_name": project,
|
||||
"note_permalink": permalink,
|
||||
}
|
||||
if expires_at is not None:
|
||||
payload["expires_at"] = _parse_expires_at(expires_at)
|
||||
|
||||
async def _create():
|
||||
try:
|
||||
config_manager = ConfigManager()
|
||||
config = config_manager.config
|
||||
host_url = config.cloud_host.rstrip("/")
|
||||
|
||||
payload: dict = {
|
||||
"project_name": project,
|
||||
"note_permalink": permalink,
|
||||
}
|
||||
if expires_at is not None:
|
||||
payload["expires_at"] = _parse_expires_at(expires_at)
|
||||
|
||||
console.print("[blue]Creating share link...[/blue]")
|
||||
|
||||
response = await make_api_request(
|
||||
@@ -122,6 +125,8 @@ def create(
|
||||
console.print("[green]Share link created successfully[/green]")
|
||||
_print_share_details(data)
|
||||
|
||||
except typer.Exit:
|
||||
raise
|
||||
except SubscriptionRequiredError as e:
|
||||
console.print("\n[red]Subscription Required[/red]\n")
|
||||
console.print(f"[yellow]{e.args[0]}[/yellow]\n")
|
||||
|
||||
@@ -142,6 +142,10 @@ class TestShareCreateCommand:
|
||||
|
||||
assert result.exit_code == 1
|
||||
assert "Invalid --expires-at" in result.stdout
|
||||
# A parse error must produce a single clean message, not a
|
||||
# spurious "Unexpected error: 1" from the broad handler
|
||||
# re-catching typer.Exit. See issue #880 review.
|
||||
assert "Unexpected error" not in result.stdout
|
||||
|
||||
def test_create_share_note_not_found(self):
|
||||
runner = CliRunner()
|
||||
|
||||
Reference in New Issue
Block a user