Skip to content

refactor(FIR-46254): merge token and url caching - #454

Open
ptiurin wants to merge 13 commits into
mainfrom
merge-caching
Open

refactor(FIR-46254): merge token and url caching#454
ptiurin wants to merge 13 commits into
mainfrom
merge-caching

Conversation

@ptiurin

@ptiurinptiurin commented Aug 22, 2025

Copy link
Copy Markdown
Contributor

This pull request introduces a significant refactor of the token caching mechanism for authentication in the Firebolt Python SDK. The main changes involve replacing the previous filesystem-based token cache with a new, encrypted, file-based cache system, updating the Auth classes to use this new cache, and removing legacy token storage code. The update also ensures that account names are consistently set in authentication objects during connection setup.

Token caching and storage refactor:

  • Introduced a new FileBasedCache class in src/firebolt/utils/cache.py that provides encrypted, persistent, file-based caching for authentication tokens and connection info, replacing the old TokenSecureStorage system. This includes logic for OS-specific cache directories, encryption, and expiry handling.
  • Updated the Auth class in src/firebolt/client/auth/base.py to use the new _firebolt_cache for token retrieval and storage, including adding an account property to ensure tokens are cached per-account.
  • Removed all references to the legacy TokenSecureStorage and related code from the authentication classes (UsernamePassword, ServiceAccount, ClientCredentials), and deleted the compatibility import in src/firebolt/common/token_storage.py.

Connection setup improvements:

  • Ensured that the account_name is consistently assigned to the auth object in both async and sync connect functions, which is necessary for correct token caching and retrieval.

Data structure enhancements:

  • Enhanced the ConnectionInfo dataclass to include a token field and a __post_init__ method for robust deserialization of nested dataclasses from cache.

Comment threadsrc/firebolt/utils/file_operations.py Fixed
Comment threadsrc/firebolt/utils/file_operations.py Fixed
@ptiurinptiurin changed the title working file cachingrefactor: merge token and url cachingAug 22, 2025
@ptiurinptiurin changed the title refactor: merge token and url cachingrefactor(FIR-46254): merge token and url cachingAug 22, 2025
@sonarqubecloud

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

@ptiurin
ptiurin marked this pull request as ready for review November 19, 2025 11:10
@ptiurin
ptiurin requested a review from a team as a code ownerNovember 19, 2025 11:10
def _token_storage(self) -> Optional[TokenSecureStorage]:
"""Token filesystem cache storage.
def _get_cached_token(self) -> Tuple[Optional[str], Optional[int]]:
"""If caching is enabled, get token from cache.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing we also return the validity of the token? What is the tuple return needed for

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment threadsrc/firebolt/utils/cache.py Outdated
Comment threadsrc/firebolt/utils/cache.py Outdated
@ptiurin

Copy link
Copy Markdown
ContributorAuthor

Created merge_caching_bkp in case some merge conflicts were not resolved correctly.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ptiurin@github-advanced-security@bogdantruta-firebolt