ghoshp83 commented on issue #3970:
URL:
https://github.com/apache/iceberg-python/issues/3970#issuecomment-5731841864
I have opened #3999 for the first half of this (the `commit_table` session
mutation), and while testing it two things came up that I did not fold into
that
PR.
**The table-scoped token never reaches the wire**
`RestCatalog` sets `session.auth = AuthManagerAdapter(self._auth_manager)`.
`requests` applies `session.auth` in `Session.prepare_request`, after the
per-request `headers` are merged, so the auth manager's `Authorization`
replaces
whatever `commit_table` put there.
Against a `requests_mock`ed catalog with `token=<catalog token>` and a table
whose config carries `{"token": "<table token>"}`, the commit request goes
out
as:
| | request `Authorization` | session `Authorization` after commit |
| -- | -- | -- |
| today | `Bearer <catalog token>` | `Bearer <table token>` |
| with the PR | `Bearer <catalog token>` | absent |
So the header assignment currently has no effect on the request, and its only
observable effect is the leak onto the session. Removing the leak does not
make
the table token apply.
Making it apply is a design question rather than a fix, which is why I left
it
out: it means deciding whether a table-scoped token should override the
configured auth manager, and if so whether that belongs in the auth manager
itself (a per-request override) or in a request-level `auth=` argument at the
call site. Happy to implement whichever you prefer, or to leave it if the
intended behaviour is that the auth manager always wins — in which case the
assignment looks like dead code worth deleting.
**The `S3FileSystem` signer sharing**
I have not touched this one. The obvious fix, passing
`skip_instance_cache=True`
when a signer is registered, would give every signed catalog its own
filesystem
instance and so give up fsspec's instance caching for those catalogs. That
is a
performance and connection-pooling trade-off I did not want to make on your
behalf. If you would rather keep the cache, the alternative is to make the
signer part of the cache key instead — for example by deriving the
`unique_id`
from the catalog's identity rather than the fixed `1925`, so two catalogs do
not
overwrite each other's registration.
Which of those would you like? I am happy to open a PR for either.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
