feat: add table-references command group - #22
Conversation
|
Good job looks great! Left a few small comments of things to look into. Please make sure you add a PR description and bump |
ClassicMMT
left a comment
ClassicMMT
left a comment
There was a problem hiding this comment.
Looking good. Almost there!
7bea2fb to
792ad95
Compare
ClassicMMT
left a comment
ClassicMMT
left a comment
There was a problem hiding this comment.
Approved. Just two last things left to make it look a bit better.
| # `datamasque.*` is exempted from mypy's `follow_imports`, so the SDK call resolves to | ||
| # `Any` here even though `list_table_references` itself declares `-> list[TableReference]`. | ||
| return cast("list[TableReference]", client.list_table_references()) |
There was a problem hiding this comment.
| # `datamasque.*` is exempted from mypy's `follow_imports`, so the SDK call resolves to | |
| # `Any` here even though `list_table_references` itself declares `-> list[TableReference]`. | |
| return cast("list[TableReference]", client.list_table_references()) | |
| references: list[TableReference] = client.list_table_references() | |
| return references |
same thing but looks way better
There was a problem hiding this comment.
Done same functional fix
| def _get_or_abort(client: DataMasqueClient, name: str) -> TableReference | None: | ||
| """Return the table reference named `name`, or `None`, or abort with the reason the lookup failed.""" | ||
| try: | ||
| return cast("TableReference | None", client.get_table_reference_by_name(name)) |
There was a problem hiding this comment.
| return cast("TableReference | None", client.get_table_reference_by_name(name)) | |
| reference: TableReference | None = client.get_table_reference_by_name(name) | |
| return reference |
or similar variable. Maybe table_reference is better?
There was a problem hiding this comment.
Kept reference over table_reference — matches this file's existing convention for a single TableReference
| dm libraries usage <name> # Show rulesets using it | ||
| ``` | ||
|
|
||
| ### Table references |
There was a problem hiding this comment.
This section explains a lot compared to the sections above and below. Could you list the commands directly for visual consistency?
Are the commands self-explanatory if the user types dm table-references --help? If not, additional usage information probably belong in SKILL.md
There was a problem hiding this comment.
Trimmed to match sibling sections — Connections/Rulesets/Libraries are all bare command blocks with inline comments, no prose. Checked dm table-references create --help directly: fully self-explanatory (examples + every flag's semantics), so nothing needs moving to SKILL.md
| @@ -1,5 +1,22 @@ | |||
| # Changelog | |||
|
|
|||
| ## Unreleased | |||
There was a problem hiding this comment.
Could we name it v.1.6.0? Then we can create a release straight after merging in
There was a problem hiding this comment.
Done renamed to v1.6.0
792ad95 to
8435e54
Compare
3 participants
Adds a
table-referencescommand group (list/get/create/update/delete) to dm, mirroring the existingconnectionscommand group, over `datamasque-python TableReferenceClient.createaccepts--file(JSON) or flags for the common case; source is a file path for file connections orschema.tablefor database connections, with format always explicit, never inferred.updatechanges only the fields passed; any CSV/format flag replacesoptionswholesale on the resulting PUT.Bumps the
datamasque-pythonfloor to>=1.2.5,<2.