chore(databases): simplify fork error description - #198
hotdata-automation[bot] wants to merge 1 commit into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| ### Changed | ||
|
|
||
| - chore(databases): simplify fork error description |
There was a problem hiding this comment.
The [Unreleased] entry omits three breaking removals that this PR makes to the public API.
Removed in this PR:
ConnectionsApi.purge_connection_cache(hotdata/api/connections_api.py)ConnectionsApi.purge_table_cache(hotdata/api/connections_api.py)CreateConnectionRequest.skip_discovery(hotdata/models/create_connection_request.py)
Failure scenario: a caller pins hotdata and upgrades past this change. Existing code calling connections_api.purge_connection_cache(connection_id) raises AttributeError at runtime. Existing code constructing CreateConnectionRequest(..., skip_discovery=True) raises a pydantic ValidationError, because skip_discovery is gone from __properties and from the model fields. Neither break appears in the changelog, and .github/workflows/release.yml builds the GitHub release notes from this section via scripts/extract-changelog.py. The release therefore ships the breaks silently.
## [0.11.0] records comparable removals under a ### Removed heading with a **Breaking:** prefix. Add the same for these three symbols.
The entry also omits the new public surface added here: DatabasesApi.get_database_lineage, DatabasesApi.lookup_database_by_name, DatabasesApi.set_database_table_constant_per_key, DatabasesApi.update_managed_table, ConnectionsApi.set_managed_table_constant_per_key, the DatabaseLineageResponse / ForkedFromInfo / LineageAncestorInfo / LineageForkInfo / ManagedTableConstantPerKeyResponse / UpdateManagedTableRequest models, and the new forked_from, created, if_not_exists, vector_precision, total_row_count and constant_per_key fields. Add an ### Added section covering them.
| search: Annotated[Optional[StrictStr], Field(description="Case-insensitive substring filter on the database name. When set, only databases whose name contains this text are returned; paging and newest-first ordering are unchanged.")] = None, | ||
| batch: Annotated[Optional[StrictStr], Field(description="List only the databases belonging to one bulk-creation batch, identified by the `batch_id` that call returned. Bulk-created databases also appear in the unfiltered listing alongside every other database; this narrows the listing to one batch. Paging works the same way, but results are ordered by database id rather than newest-first, because every database in a batch is created at once.")] = None, | ||
| database_id: Annotated[StrictStr, Field(description="Database ID")], | ||
| forks_limit: Annotated[Optional[Annotated[int, Field(le=100, strict=True, ge=1)]], Field(description="How many of the databases forked from this one to list (1-100, default 25). Values outside the range are clamped. `fork_count` always reports the true total, whatever this is set to.")] = None, |
There was a problem hiding this comment.
nit: the forks_limit description promises clamping, but this client rejects out-of-range values instead (not blocking).
@validate_call on get_database_lineage enforces Field(le=100, ge=1). A caller passing forks_limit=500 gets a pydantic ValidationError and no HTTP request. The server-side clamping the text describes is unreachable through the SDK.
The text comes from the OpenAPI spec, so fix it upstream in hotdata-dev/www: either drop the clamping sentence, or drop the minimum/maximum on the parameter so the client forwards the value.
There was a problem hiding this comment.
Review
Context disclosure: the full diff was omitted from the review prompt because it is 452KB. gh pr diff returned more than one read allows. Review used the changed-file list plus Read/rg over the checkout at the head SHA. Files read: CHANGELOG.md, pyproject.toml, hotdata/__init__.py, hotdata/models/__init__.py, hotdata/api/connections_api.py, hotdata/api/databases_api.py, hotdata/models/database_lineage_response.py, hotdata/models/forked_from_info.py, hotdata/models/update_managed_table_request.py, hotdata/models/create_database_response.py, tests/integration/test_connections_read.py, .github/workflows/regenerate.yml, .github/workflows/check-release.yml, and the diff hunks for every changed model.
Blocking Issues
CHANGELOG.mdline 12 omits three breaking removals.ConnectionsApi.purge_connection_cache,ConnectionsApi.purge_table_cacheandCreateConnectionRequest.skip_discoveryare gone..github/workflows/release.ymlbuilds release notes from this section, so a release ships the breaks with no notice. Details in the inline comment.
Action Required
Add a Removed block under [Unreleased] listing the three removed symbols with the **Breaking:** prefix that [0.11.0] uses. Add an Added block for the new lineage and constant-per-key surface.
Non-blocking
tests/integration/test_connections_read.py line 32 calls connections_api.purge_connection_cache(connection_id), which this PR removes. The test carries @pytest.mark.skip, so collection and CI are unaffected today. The skip reason says to re-enable the test once the seeded connection is restored. Re-enabling it then raises AttributeError. Delete line 32 and the "cache purge" clause in the module docstring on line 4. This file is outside the diff, so no inline comment anchors to it.
CI
The status snapshot shows Integration Tests / scenario-parity passing. check-release, Integration Tests / unit and Integration Tests / integration were queued or running at review time, so no conclusion is drawn from them here.
Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www/pull/431