Skip to content

Commit cea1d75

Browse files
rustyconoverclaude
andcommitted
Add attach_id_required field and update CLI to use --attach-id/--catalog options
- Add attach_id_required field to CatalogAttachResult for stateful/stateless catalog indication - Update ReadOnlyCatalogInterface to return attach_id_required=False - Replace positional attach_id arguments with --attach-id and --catalog options across all catalog CLI commands (schema, table, view, transaction, version) - Add get_attach_id_from_options() helper for auto-attach workflow - Show warning when using --catalog with stateful catalogs - Update and add CLI tests for new option syntax and validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent fcf7bfe commit cea1d75

9 files changed

Lines changed: 658 additions & 135 deletions

File tree

tests/catalog/test_serialization.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,12 +346,13 @@ class TestArrowSchemaCorrectness:
346346
def test_catalog_attach_result_schema(self) -> None:
347347
"""Verify CatalogAttachResult Arrow schema."""
348348
schema = CatalogAttachResult.ARROW_SCHEMA
349-
assert len(schema) == 5
349+
assert len(schema) == 6
350350
assert schema.field("attach_id").type == pa.binary()
351351
assert schema.field("supports_transactions").type == pa.bool_()
352352
assert schema.field("supports_time_travel").type == pa.bool_()
353353
assert schema.field("catalog_version_frozen").type == pa.bool_()
354354
assert schema.field("catalog_version").type == pa.int64()
355+
assert schema.field("attach_id_required").type == pa.bool_()
355356

356357
def test_schema_info_schema(self) -> None:
357358
"""Verify SchemaInfo Arrow schema."""

tests/client/test_cli_catalog_functions.py

Lines changed: 128 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ def test_catalog_attach_returns_attach_id(self, example_worker: str) -> None:
5454
assert len(attach_result["attach_id"]) > 0
5555
assert attach_result["supports_transactions"] is False
5656
assert attach_result["catalog_version_frozen"] is True
57+
# ReadOnlyCatalogInterface returns attach_id_required=False
58+
assert attach_result["attach_id_required"] is False
5759

5860

5961
class TestCLISchemaContents:
@@ -72,15 +74,47 @@ def test_schema_contents_lists_functions(self, example_worker: str) -> None:
7274
attach_data = json.loads(attach_result.output)
7375
attach_id = attach_data["attach_id"]
7476

75-
# List schema contents (now nested under catalog)
77+
# List schema contents using --attach-id option
7678
contents_result = runner.invoke(
7779
cli,
7880
[
7981
"catalog",
8082
"schema",
8183
"contents",
84+
"main",
85+
"--attach-id",
8286
attach_id,
87+
"--server",
88+
example_worker,
89+
],
90+
)
91+
assert contents_result.exit_code == 0
92+
93+
# Parse all output lines as JSON
94+
lines = contents_result.output.strip().split("\n")
95+
items = [json.loads(line) for line in lines if line.strip()]
96+
97+
# Should have items
98+
assert len(items) > 0
99+
100+
# All items should be functions
101+
for item in items:
102+
assert item["type"] == "function"
103+
104+
def test_schema_contents_with_catalog_option(self, example_worker: str) -> None:
105+
"""Schema contents works with --catalog option for auto-attach."""
106+
runner = CliRunner()
107+
108+
# List schema contents using --catalog option (auto-attach)
109+
contents_result = runner.invoke(
110+
cli,
111+
[
112+
"catalog",
113+
"schema",
114+
"contents",
83115
"main",
116+
"--catalog",
117+
"example",
84118
"--server",
85119
example_worker,
86120
],
@@ -110,15 +144,16 @@ def test_all_example_functions_present(self, example_worker: str) -> None:
110144
assert attach_result.exit_code == 0
111145
attach_id = json.loads(attach_result.output)["attach_id"]
112146

113-
# Get schema contents (now nested under catalog)
147+
# Get schema contents using --attach-id option
114148
contents_result = runner.invoke(
115149
cli,
116150
[
117151
"catalog",
118152
"schema",
119153
"contents",
120-
attach_id,
121154
"main",
155+
"--attach-id",
156+
attach_id,
122157
"--server",
123158
example_worker,
124159
],
@@ -158,8 +193,9 @@ def test_function_info_has_correct_structure(self, example_worker: str) -> None:
158193
"catalog",
159194
"schema",
160195
"contents",
161-
attach_id,
162196
"main",
197+
"--attach-id",
198+
attach_id,
163199
"--server",
164200
example_worker,
165201
],
@@ -195,8 +231,9 @@ def test_scalar_functions_have_correct_type(self, example_worker: str) -> None:
195231
"catalog",
196232
"schema",
197233
"contents",
198-
attach_id,
199234
"main",
235+
"--attach-id",
236+
attach_id,
200237
"--server",
201238
example_worker,
202239
],
@@ -229,8 +266,9 @@ def test_table_functions_have_correct_type(self, example_worker: str) -> None:
229266
"catalog",
230267
"schema",
231268
"contents",
232-
attach_id,
233269
"main",
270+
"--attach-id",
271+
attach_id,
234272
"--server",
235273
example_worker,
236274
],
@@ -262,14 +300,96 @@ def test_schema_list_shows_main(self, example_worker: str) -> None:
262300
)
263301
attach_id = json.loads(attach_result.output)["attach_id"]
264302

265-
# List schemas (now nested under catalog)
303+
# List schemas using --attach-id option
304+
list_result = runner.invoke(
305+
cli,
306+
[
307+
"catalog",
308+
"schema",
309+
"list",
310+
"--attach-id",
311+
attach_id,
312+
"--server",
313+
example_worker,
314+
],
315+
)
316+
assert list_result.exit_code == 0
317+
318+
# Parse schema info
319+
schema_info = json.loads(list_result.output)
320+
assert schema_info["name"] == "main"
321+
assert schema_info["is_default"] is True
322+
323+
def test_schema_list_with_catalog_option(self, example_worker: str) -> None:
324+
"""Schema list works with --catalog option."""
325+
runner = CliRunner()
326+
327+
# List schemas using --catalog option (auto-attach)
266328
list_result = runner.invoke(
267329
cli,
268-
["catalog", "schema", "list", attach_id, "--server", example_worker],
330+
[
331+
"catalog",
332+
"schema",
333+
"list",
334+
"--catalog",
335+
"example",
336+
"--server",
337+
example_worker,
338+
],
269339
)
270340
assert list_result.exit_code == 0
271341

272342
# Parse schema info
273343
schema_info = json.loads(list_result.output)
274344
assert schema_info["name"] == "main"
275345
assert schema_info["is_default"] is True
346+
347+
348+
class TestCLIAttachIdCatalogOptions:
349+
"""Tests for --attach-id and --catalog option validation."""
350+
351+
def test_mutual_exclusivity(self, example_worker: str) -> None:
352+
"""Error when both --attach-id and --catalog are specified."""
353+
runner = CliRunner()
354+
355+
# First get an attach_id
356+
attach_result = runner.invoke(
357+
cli,
358+
["catalog", "attach", "example", "--server", example_worker],
359+
)
360+
attach_id = json.loads(attach_result.output)["attach_id"]
361+
362+
# Try to use both options
363+
result = runner.invoke(
364+
cli,
365+
[
366+
"catalog",
367+
"schema",
368+
"list",
369+
"--attach-id",
370+
attach_id,
371+
"--catalog",
372+
"example",
373+
"--server",
374+
example_worker,
375+
],
376+
)
377+
assert result.exit_code != 0
378+
assert "Cannot specify both" in result.output
379+
380+
def test_requires_attach_id_or_catalog(self, example_worker: str) -> None:
381+
"""Error when neither --attach-id nor --catalog is specified."""
382+
runner = CliRunner()
383+
384+
result = runner.invoke(
385+
cli,
386+
[
387+
"catalog",
388+
"schema",
389+
"list",
390+
"--server",
391+
example_worker,
392+
],
393+
)
394+
assert result.exit_code != 0
395+
assert "Must specify either" in result.output

vgi/catalog/catalog_interface.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ class CatalogAttachResult:
4040
# The initial catalog version, it increments when schemas, tables
4141
# or other objects change.
4242
catalog_version: int
43+
# Indicate if the attach_id must be persisted across commands.
44+
# True: Catalog is stateful; attach_id represents a session
45+
# False: Catalog is stateless; CLI can auto-attach on each command
46+
attach_id_required: bool = True
4347

4448
ARROW_SCHEMA: ClassVar[pa.Schema] = pa.schema(
4549
[
@@ -48,6 +52,7 @@ class CatalogAttachResult:
4852
pa.field("supports_time_travel", pa.bool_(), nullable=False),
4953
pa.field("catalog_version_frozen", pa.bool_(), nullable=False),
5054
pa.field("catalog_version", pa.int64(), nullable=False),
55+
pa.field("attach_id_required", pa.bool_(), nullable=False),
5156
] # type: ignore[arg-type]
5257
)
5358

@@ -61,6 +66,7 @@ def serialize(self) -> bytes:
6166
"supports_time_travel": self.supports_time_travel,
6267
"catalog_version_frozen": self.catalog_version_frozen,
6368
"catalog_version": self.catalog_version,
69+
"attach_id_required": self.attach_id_required,
6470
}
6571
],
6672
schema=self.ARROW_SCHEMA,
@@ -79,6 +85,7 @@ def deserialize(cls, batch: pa.RecordBatch) -> Self:
7985
"supports_time_travel",
8086
"catalog_version_frozen",
8187
"catalog_version",
88+
"attach_id_required",
8289
],
8390
)
8491
return cls(
@@ -87,6 +94,7 @@ def deserialize(cls, batch: pa.RecordBatch) -> Self:
8794
supports_time_travel=row["supports_time_travel"],
8895
catalog_version_frozen=row["catalog_version_frozen"],
8996
catalog_version=row["catalog_version"],
97+
attach_id_required=row["attach_id_required"],
9098
)
9199

92100

@@ -959,6 +967,7 @@ def catalog_attach(
959967
supports_time_travel=False,
960968
catalog_version_frozen=True,
961969
catalog_version=1,
970+
attach_id_required=False,
962971
)
963972

964973
def schema_get(

vgi/client/cli_catalog.py

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
├── detach <attach_id> # Detach from a catalog
99
├── create <name> # Create a new catalog
1010
├── drop <name> # Drop a catalog
11-
├── version <attach_id> # Get catalog version
11+
├── version # Get catalog version (--attach-id or --catalog)
1212
├── schema # Schema operations
1313
│ ├── list/get/create/drop/contents
1414
├── table # Table operations
@@ -29,7 +29,9 @@
2929
from vgi.client.cli_table import table
3030
from vgi.client.cli_transaction import transaction
3131
from vgi.client.cli_utils import (
32+
bytes_to_hex,
3233
catalog_attach_result_to_dict,
34+
get_attach_id_from_options,
3335
hex_to_attach_id,
3436
hex_to_transaction_id,
3537
output_json,
@@ -124,23 +126,37 @@ def catalog_drop(name: str, server: str) -> None:
124126

125127

126128
@catalog.command("version")
127-
@click.argument("attach_id")
129+
@click.option("--attach-id", help="Hex-encoded attach ID")
130+
@click.option("--catalog", "catalog_name", help="Catalog name for auto-attach")
131+
@click.option("--attach-options", default="{}", help="Attach options as JSON")
128132
@click.option("--server", required=True, help="VGI worker command")
129133
@click.option("--transaction-id", help="Transaction ID (hex) for transactional read")
130-
def catalog_version(attach_id: str, server: str, transaction_id: str | None) -> None:
131-
"""Get the current catalog version.
132-
133-
ATTACH_ID is the hex-encoded attach ID from catalog attach.
134-
135-
"""
134+
def catalog_version(
135+
attach_id: str | None,
136+
catalog_name: str | None,
137+
attach_options: str,
138+
server: str,
139+
transaction_id: str | None,
140+
) -> None:
141+
"""Get the current catalog version."""
136142
client = Client(server)
143+
opts = parse_json_option(attach_options, "--attach-options")
144+
resolved_attach_id, is_stateful = get_attach_id_from_options(
145+
client, attach_id, catalog_name, opts
146+
)
147+
if is_stateful and catalog_name:
148+
click.echo(
149+
"Warning: Using --catalog with a stateful catalog. "
150+
"Consider using --attach-id for session persistence.",
151+
err=True,
152+
)
137153
version = client.catalog_version(
138-
attach_id=hex_to_attach_id(attach_id),
154+
attach_id=resolved_attach_id,
139155
transaction_id=(
140156
hex_to_transaction_id(transaction_id) if transaction_id else None
141157
),
142158
)
143-
output_json({"version": version, "attach_id": attach_id})
159+
output_json({"version": version, "attach_id": bytes_to_hex(resolved_attach_id)})
144160

145161

146162
# Add nested subcommand groups

0 commit comments

Comments
 (0)