chore: disallow additional properties in table request schemas - #189
chore: disallow additional properties in table request schemas#189hotdata-automation[bot] wants to merge 1 commit into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| @validate_call | ||
| def purge_table_cache( | ||
| def set_managed_table_constant_per_key( |
There was a problem hiding this comment.
This PR deletes purge_connection_cache and purge_table_cache, but tests/integration/test_connections_read.py:32 still calls connections_api.purge_connection_cache(connection_id). Update that scenario test in this PR: drop the purge step and the "cache purge" wording in the module docstring at line 4.
Failure scenario: a developer removes the @pytest.mark.skip at tests/integration/test_connections_read.py:15 (added for an unrelated degraded fixture) and the test fails with AttributeError: 'ConnectionsApi' object has no attribute 'purge_connection_cache', not with the API failure it was written to catch. CI stays green today only because the test is skipped.
tests/integration/test_managed_tables_lifecycle.py:17 also names purge_table_cache in a comment, which is now a method that does not exist.
|
|
||
| ### Changed | ||
|
|
||
| - chore: disallow additional properties in table request schemas |
There was a problem hiding this comment.
Record the removed public API under ### Removed with a **Breaking:** marker. This PR removes three public symbols and the changelog reports none of them:
ConnectionsApi.purge_connection_cacheConnectionsApi.purge_table_cacheCreateConnectionRequest.skip_discovery
The ### Removed block at CHANGELOG.md lines 10-20 sets the convention: every prior endpoint and model removal in this same [Unreleased] section carries a **Breaking:** entry.
Failure scenario: a consumer upgrades from 0.10.0, reads a changelog that lists only doc and schema changes, and their purge_connection_cache(...) call raises AttributeError at runtime with no prior warning. A CreateConnectionRequest(..., skip_discovery=True) call raises ValidationError the same way.
| """ # noqa: E501 | ||
| columns: Optional[List[ColumnInfo]] = None | ||
| connection: StrictStr | ||
| constant_per_key: List[StrictStr] = Field(description="Columns the table declares constant for a given key: for every row, any other row sharing its key holds the same value of these columns. Declaring this lets a keyed mutation narrow its search for prior versions to the values the upload carries. Empty when none is declared, which is the unrestricted search. Unlike `partition_by` and `sorted_by` this is NOT fixed at creation — it changes only which files a mutation opens, never how rows are written — so read it here rather than assuming a declaration took effect.") |
There was a problem hiding this comment.
nit: add a changelog note that constant_per_key is required on TableInfo (not blocking).
constant_per_key carries no default, so code that constructs TableInfo directly — test mocks and fixtures — must now supply it. The 0.9.0 entry at CHANGELOG.md lines 65-73 called out exactly this for partition_by and sorted_by.
Read-only consumers are unaffected, so this is a documentation gap rather than a defect.
There was a problem hiding this comment.
Review
The full-diff block was omitted from the review context because the patch is too large. The patch was fetched with gh pr diff 189 and read in full. Files also read from the checkout: CHANGELOG.md, pyproject.toml, tests/integration/test_connections_read.py, .github/workflows/integration-tests.yml, .github/workflows/check-release.yml, README.md, hotdata/models/table_info.py, hotdata/api/connections_api.py.
Blocking Issues
tests/integration/test_connections_read.py:32callsconnections_api.purge_connection_cache, which this PR removes. The test is skipped, so CI does not catch the dangling reference.tests/integration/test_managed_tables_lifecycle.py:17namespurge_table_cachein a comment.CHANGELOG.md:24records no removal entries. This PR removesConnectionsApi.purge_connection_cache,ConnectionsApi.purge_table_cache, andCreateConnectionRequest.skip_discovery. The Removed block at CHANGELOG.md:10-20 shows the convention for these.
Action Required
- Update
tests/integration/test_connections_read.py: remove the purge step and the 'cache purge' wording in the module docstring. Update the stale comment intests/integration/test_managed_tables_lifecycle.py. - Add
**Breaking:**entries under Removed for the three removed public symbols.
Note on scope
The title and description describe only a schema change: 'disallow additional properties in table request schemas'. The patch also removes two endpoints, adds three DatabasesApi methods (get_database_lineage, lookup_database_by_name, set_database_table_constant_per_key), adds ConnectionsApi.set_managed_table_constant_per_key, and adds six models. Widen the description so a reader can see the removals without reading the diff.
CI status at review time: all five checks were queued or in progress. No check result is reported here.
Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www/pull/421