fix(managed): adopt hotdata 0.6.0 native x_database_id result params#36
Merged
Conversation
The 0.6.0 SDK (bumped in #34) added x_database_id to get_result, get_query_run, and get_result_arrow — and made it REQUIRED on get_result_arrow, so the header-based scope from #35 would raise TypeError at the Arrow fetch under the new SDK (the mocked tests could not see a generated-signature change). - _wait_result_ready / _await_query_run / _fetch_result_arrow pass x_database_id natively; the scoped default-header workaround is gone. - hotdata pin raised to >=0.6.0 (older framework code cannot run on 0.6.0 anyway, per the required param). - Test fakes mirror the 0.6.0 signatures, with x_database_id required on the Arrow fake so dropping the scope fails the suite again. Verified live against api.hotdata.dev: the state-table append path (fetch -> concat -> load) completes on SDK 0.6.0.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| - `ManagedDatabaseClient.fetch_table` now carries the `X-Database-Id` scope header on the result poll, the query-run poll, and the Arrow fetch — not only on the query submit. Results of database-scoped queries are themselves database-scoped, so every read against an existing synced table (merge/append loads, dlt state restore) failed with `400: Bad Request` once the table had data (dlthubworker#70). | |||
There was a problem hiding this comment.
super nit: (not blocking) This "Fixed" entry still describes the scope as being carried "on the ... header," but with this PR shipping in the same Unreleased section, it now rides the native x_database_id parameters rather than a header. Consider rewording to "carries the database scope" so it stays consistent with the new "Changed" entry below.
There was a problem hiding this comment.
LGTM. Correctly adopts the native x_database_id result params from hotdata 0.6.0, drops the header workaround, and updates tests to guard against dropping the scope. Dependency bump is consistent across pyproject.toml and uv.lock. One super nit on the changelog wording.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 0.6.0 SDK (bumped in #34) added x_database_id to get_result,
get_query_run, and get_result_arrow — and made it REQUIRED on
get_result_arrow, so the header-based scope from #35 would raise
TypeError at the Arrow fetch under the new SDK (the mocked tests could
not see a generated-signature change).
x_database_id natively; the scoped default-header workaround is gone.
0.6.0 anyway, per the required param).
the Arrow fake so dropping the scope fails the suite again.
Verified live against api.hotdata.dev: the state-table append path
(fetch -> concat -> load) completes on SDK 0.6.0.