chore(databases): clarify fork behavior and error cases - #193
hotdata-automation[bot] wants to merge 1 commit into
Conversation
|
|
||
| ### Changed | ||
|
|
||
| - chore(databases): clarify fork behavior and error cases |
There was a problem hiding this comment.
Add the breaking removals to the ### Removed block above.
This regeneration drops three public symbols:
ConnectionsApi.purge_connection_cache(hotdata/api/connections_api.py)ConnectionsApi.purge_table_cache(same file)CreateConnectionRequest.skip_discovery(hotdata/models/create_connection_request.py:34)
Only a ### Changed line lands here. The [Unreleased] ### Removed block at CHANGELOG.md:10-20 records every earlier breaking removal with a **Breaking:** marker, so this release will ship three undocumented ones.
Failure scenario for skip_discovery: CreateConnectionRequest sets no extra in its model_config, so pydantic v2 applies its default of extra="ignore". A caller upgrading and still passing CreateConnectionRequest(config=..., name=..., source_type=..., skip_discovery=True) gets no error and no warning. The field is dropped from the request body, the server runs discovery, and the caller reads the release notes and finds nothing about it.
There was a problem hiding this comment.
Review
The Full diff block was omitted from the review prompt as too large. The whole patch was fetched with gh pr diff 193. Files read from the checkout: CHANGELOG.md, hotdata/models/create_connection_request.py, hotdata/models/forked_from_info.py, hotdata/models/database_lineage_response.py, hotdata/models/update_managed_table_request.py, hotdata/models/managed_table_constant_per_key_response.py, hotdata/api/databases_api.py, hotdata/api/connections_api.py, test/test_connections_api.py, test/test_database_lineage_response.py, tests/integration/test_connections_read.py, hotdata/uploads.py, .github/workflows/integration-tests.yml, .github/workflows/check-release.yml.
The generated models, serializers, and __init__ exports for the six new types match the generator's output and are consistent. No correctness problem was found in them.
Blocking Issues
CHANGELOG.md:24 — three breaking removals are not recorded.
This regeneration removes ConnectionsApi.purge_connection_cache, ConnectionsApi.purge_table_cache, and CreateConnectionRequest.skip_discovery. Only a Changed line is added. The [Unreleased] Removed block at CHANGELOG.md:10-20 records every earlier breaking removal with a **Breaking:** marker.
skip_discovery fails silently. CreateConnectionRequest.model_config sets no extra, so pydantic v2 uses extra="ignore". A caller still passing skip_discovery=True after upgrading gets no error, the field is dropped from the request body, and the server runs discovery anyway.
Action Required
Add a Removed entry under [Unreleased] naming the two ConnectionsApi methods and the CreateConnectionRequest.skip_discovery field, each marked **Breaking:**, matching the format of the entries at CHANGELOG.md:12-20.
Nit (not blocking)
tests/integration/test_connections_read.py:32 calls connections_api.purge_connection_cache(connection_id), which this PR removes. That test carries @pytest.mark.skip at line 15, so pytest never runs the body and CI stays green. Whoever lifts the skip gets an AttributeError instead of the connection failure the skip reason describes. Drop the call and the cache-purge mention in the module docstring at line 4.
CI
Every check listed at review start was still queued or in progress: Check release metadata / Verify changelog matches version bump, Integration Tests / integration, Integration Tests / scenario-parity, Integration Tests / unit. No check result was available, so this review makes no claim about test outcomes.
Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www/pull/427