ci: validate typed Python DataSource failure head - #3
Closed
Robin-Everaars wants to merge 9 commits into
Closed
Conversation
Add an optional bearer_access_token_file to the Iceberg REST catalog config. When the path is set, the token is read from that file and re-read when the file changes or a short refresh interval elapses, so a rotated projected service account token is picked up without restarting the server. The file takes precedence over the static bearer_access_token. main already resolves credentials per request through CatalogCredentials::retrieve(), so this adds a FileCatalogCredentials implementation plus the config field and the wiring in create_catalog_manager. No provider changes are needed. Closes lakehq#2287.
An empty or whitespace-only token file now yields an error instead of caching an empty credential that would be sent as a bare bearer header. The empty read is never cached, so the next retrieve picks up the token as soon as the file holds one again. The Iceberg REST credentials selection moved into iceberg_rest_credentials with behavioral tests for the file precedence, the static and OAuth fallbacks and the no-token case, covering the previously untested wiring. The token file path no longer takes a needless clone.
…e per request The token file credential kept an in-memory cache guarded by an mtime check, a TTL and a mutex, yet still stat'd the file on every call, so the cache added state without a filesystem-free fast path. Drop the cache: retrieve() now reads the file, trims it and errors on an empty token, so every read is fresh. Because IcebergRestCatalogProvider resolved the credential once and baked the Authorization header into a single ApiClient, drop_database(cascade) reused that client across every list and drop request. A projected service account token that rotated partway through the cascade kept sending the stale header, and because the per-object drop errors are ignored the cascade could partially apply. Add a with_auth_retry helper that builds the client from a freshly read credential, runs the request and, on a 401, rebuilds the client and retries the request once. Every catalog operation goes through it, including each request in the cascade loop, so a mid-cascade rotation is recovered per request. The one-time startup config fetch is the exception: it caches its result and a 401 there is a hard startup failure that a same-token retry cannot recover. The shared reqwest::Client and its connection pool are reused across attempts. Cover the helper with unit tests (retry once on 401, no second retry, no retry on other errors) and add a mock server regression test where the server rejects the old token mid-cascade while the token file rotates, and the cascade drop completes with every request eventually authorized.
Add bearer_access_token_file to the Iceberg REST catalog options and an example, noting that the token is re-read per request and that a request rejected with a 401 is retried once after reloading the file.
The CASCADE path wrapped the table and view listings in `if let Ok(...)` and each per-object drop in `let _ =`, so any failure was silently discarded. A transient error on a table drop or a listing produced a half applied cascade that still dropped the namespace and reported success. Every list_tables, list_views, drop_table and drop_view failure now propagates as an error. Only benign statuses are tolerated: NOT_FOUND on a per-object drop (a concurrent removal), NOT_FOUND on a listing (the namespace is already gone, handled by the trailing if_exists drop) and 405 or 501 on list_views (a catalog with no views endpoint, which the old code also tolerated). Signed-off-by: HOIST IT B.V. <info@hoist-it.nl>
Signed-off-by: HOIST IT B.V. <info@hoist-it.nl>
Signed-off-by: Robin Everaars <robin.everaars@hoist-it.nl>
Signed-off-by: Robin Everaars <robin.everaars@hoist-it.nl>
Signed-off-by: Robin Everaars <robin.everaars@hoist-it.nl>
|
Warning The baseline was not found. Ibis Test ReportCommit Information
Test Summary
Test DetailsError CountsPassed Tests Diff(empty) Failed Tests |
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.
Temporary CI-only pull request for exact-head verification of #1.
Do not merge this branch. The integration target remains
integration/dp-enginein #1.