fix(rest): keep table tokens off the catalog session - #3976
Open
jaideeppyne wants to merge 1 commit into
Open
jaideeppyne wants to merge 1 commit into
jaideeppyne wants to merge 1 commit into
Conversation
commit_table wrote a table-scoped Authorization header into the live session mapping, so later requests from the same RestCatalog carried it. Send that token on the commit request only. Also skip fsspec's S3FileSystem instance cache when a custom signer is registered, so two catalogs cannot overwrite each other's before-sign handler. Closes apache#3970
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.
Rationale for this change
RestCatalog.commit_tableassigned intoself._session.headers, which is the session's own mapping. A table-scoped token then stuck on the catalog session for later requests.Session.auth would also overwrite a per-request
Authorizationheader with the catalog token, so the table token is applied through a request-local auth callable and the session headers are left unchanged.A second, related leak: fsspec caches
S3FileSysteminstances by constructor kwargs. Custom S3 signers are registered after construction under a fixed event id, so a second catalog with the same client kwargs replaced the first catalog's signer.skip_instance_cacheis set when a signer is in use.Closes #3970
Are these changes tested?
Unit tests in
tests/catalog/test_rest.pyandtests/io/test_fsspec.py.Are there any user-facing changes?
Bug fix. Catalog sessions no longer retain a table-scoped token after
commit_table. FileIO instances with custom S3 signers are no longer shared across catalogs via fsspec's instance cache.