[Storage] added SessionProvider to create session - #48756
[Storage] added SessionProvider to create session#48756anjaliratnam-msft wants to merge 1 commit into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
| StorageSensitiveHeaderCleanupPolicy(**kwargs), | ||
| ] | ||
| use_session = bool(kwargs.pop("use_session", False)) | ||
| session_options = kwargs.pop("session_options", None) |
There was a problem hiding this comment.
We should not take an options bag, instead just take each individual option as an individual kwarg. In this case session_provider.
|
|
||
| session_provider = getattr(session_options, "session_provider", None) | ||
| if session_provider is None: | ||
| from .session_provider import ContainerSessionProvider # module-level import would cycle |
There was a problem hiding this comment.
Would it create a cycle?
| if account_name is None: | ||
| raise ValueError( | ||
| "Unable to determine the account name from the service URL. " | ||
| "Supply session_options.account_name when using a custom endpoint." |
There was a problem hiding this comment.
We should handle account name a little differently than other languages. Will discuss offline.
| :keyword str audience: The audience to use when requesting tokens for Azure Active Directory | ||
| authentication. Only has an effect when credential is of type TokenCredential. The value could be | ||
| https://storage.azure.com/ (default) or https://<account>.blob.core.windows.net. | ||
| :keyword bool use_session: If True, enable session-based authentication for this container. |
There was a problem hiding this comment.
Will need to add session provider argument anywhere we have this one.
| self.position = position | ||
|
|
||
|
|
||
| class SessionOptions(object): |
There was a problem hiding this comment.
As mentioned before, we should just get rid of this.
Also, a little note if we were to keep this, we don't need to inherit from object anymore. That is something we had to do for older Python versions so you may see remnants in the code (and AI may pick up on them) but for any new classes, we don't need to do this.
| use_session = bool(kwargs.pop("use_session", False)) | ||
| session_options = kwargs.pop("session_options", None) | ||
| if use_session: | ||
| if not hasattr(credential, "get_token"): |
There was a problem hiding this comment.
Let's let the session provider constructor handle this check now.
| return status == 400 and error_code == StorageErrorCode.FEATURE_NOT_ENABLED | ||
|
|
||
|
|
||
| class ContainerSessionProvider: |
There was a problem hiding this comment.
Jus tone file. Put everything in session.py
2 participants
Summary
This PR introduces SessionProvider building off the previous session PR. Customers can optionally supply one via session_options.session_provider and reuse it across clients to share the cache. If it's not supplied, one is created and scoped to the client, so existing behavior is unchanged.
Not in this PR