feat: full CRUD coverage for all Dremio objects#2
Conversation
Adds consistent create/update/delete operations so users never need to guess whether to use a CLI subcommand or raw SQL for a given object. New command groups: - drs engine (list/get/create/update/delete/enable/disable) - drs user (list/get/invite/update/delete) - drs role (list/get/create/update/delete) - drs grant (get/set/remove) Extended existing groups: - catalog: create-space, create-folder, delete - schema: set-wiki, set-tags - reflect: create Client: added _put helper, catalog mutations, wiki/tag setters, reflection creation, engine CRUD, user/role CRUD, grant management. 47 commands total (up from 21), 105 tests (up from 73). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a91c155f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/drs/client.py
Outdated
| async def _put(self, url: str, json: dict | None = None) -> Any: | ||
| resp = await self._client.put(url, json=json) | ||
| resp.raise_for_status() | ||
| return resp.json() |
There was a problem hiding this comment.
Handle empty PUT responses before JSON decoding
The new _put helper always returns resp.json() after raise_for_status(), but several newly added flows call PUT for control operations (for example engine enable/disable and grant updates) where a successful response can be empty; when that happens, resp.json() raises JSONDecodeError and the CLI reports a failure even though the API call succeeded. This should mirror _delete by checking resp.content before decoding.
Useful? React with 👍 / 👎.
src/drs/commands/schema.py
Outdated
| except httpx.HTTPStatusError: | ||
| pass |
There was a problem hiding this comment.
Propagate non-404 prefetch errors in schema updates
set_wiki swallows every HTTPStatusError from the pre-read (get_wiki) and falls back to version=None (and set_tags repeats the same pattern), which means authorization/server failures are treated like “no existing wiki/tags” and the write proceeds without the expected concurrency version. In cases like transient 5xx or permission issues, this can cause misleading behavior and accidental overwrite attempts; only 404 should be ignored while other statuses should be surfaced via handle_api_error.
Useful? React with 👍 / 👎.
…s only swallow 404 P1: _put now checks resp.content before JSON decode, matching _delete behavior. Prevents JSONDecodeError on engine enable/disable and grant updates that return empty 200 responses. P2: set_wiki/set_tags prefetch now only catches 404 (no existing wiki/tags). Auth failures and 5xx are propagated via handle_api_error instead of being silently ignored. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…consistent CRUD verbs - Binary: drs → dremio - catalog → folder (create uses SQL: CREATE SPACE / CREATE FOLDER) - reflect → reflection (drop → delete, status → get) - jobs → job - access commands split: whoami/audit → user, grants → folder - schema wiki/tags split into separate wiki and tag groups - set → update, remove → delete for consistent CRUD verbs - search promoted to top-level command - Delete old command files (catalog, reflect, access, jobs) - Update README with new commands, install instructions, CRUD matrix 107 tests passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… stale docs - Retry up to 3 times on timeouts and 429/502/503/504 with 1s/2s/4s backoff - No retry on client errors (4xx except 429) - Remove ARCHITECTURE_REVIEW.md and ONE_PAGER.md (stale spike artifacts) - 8 new retry tests (115 total passing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Regenerate corrupted uv.lock (TOML parse error at pre-commit entry) - Fix all ruff lint violations: StrEnum, Optional→X|None, import order, collections.abc, contextlib.suppress, Path.open - Apply ruff format to all files - Fix SSE parser to flush leftover buf content when stream closes without trailing newline (review comment dremio#2) - Add conversations key fallback in _chat_output so payloads shaped as {"conversations": [...]} render correctly (review comment dremio#1) - Add test for SSE no-trailing-newline edge case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
New command groups
drs enginedrs userdrs roledrs grantExtended existing groups
drs catalogcreate-space,create-folder,deletedrs schemaset-wiki,set-tagsdrs reflectcreateOther changes in this branch
/v0/projects/{pid}/...)sys.project.jobs,sys.project.reflections)Design principle
All mutations go through the REST API, not SQL. The only exception is DDL for tables/views (CREATE VIEW, ALTER TABLE, etc.) which is inherently SQL — use
drs query runfor those.Test plan
uv run pytest tests/ -q)drs describe <command>returns correct schemas for all 47 commands--dry-runflag on all destructive operations🤖 Generated with Claude Code