refactor(rest): extract SigV4 signing into catalog/rest/sigv4.py#3446
refactor(rest): extract SigV4 signing into catalog/rest/sigv4.py#3446adamdavis99 wants to merge 2 commits into
Conversation
962ca91 to
ec9bb30
Compare
| from typing import Any | ||
| from urllib import parse | ||
|
|
||
| import boto3 |
There was a problem hiding this comment.
Could we move the urllib and boto imports to be lazily loaded inside the function similar to the older function had it in __init__.py?
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
2 participants
Closes #3410
Rationale for this change
Extracts
SigV4Adapterand related signing constants out of the nested closure insideRestCatalog._init_sigv4into a dedicated modulepyiceberg/catalog/rest/sigv4.py. Separating concerns makes the signing logic easier to evolve, test, and extend independently of the catalog (e.g., retry config improvements tracked in #3008).Are these changes tested?
Yes. The existing 9 sigv4 tests in
tests/catalog/test_rest.pycoversigv4.pyat 100%. No new tests were added as this is a behavior-preserving refactor.Are there any user-facing changes?
SigV4Adapteris now a module-level class and explicitly re-exported frompyiceberg.catalog.rest. All existing imports continue to work unchanged. The previously privateRestCatalog._init_sigv4method has been removed (it was not part of the public API).