Skip to content

[Storage] [Named Keywords] Blob Package #41726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

weirongw23-msft
Copy link
Member

No description provided.

@weirongw23-msft weirongw23-msft marked this pull request as ready for review June 23, 2025 21:24
@Copilot Copilot AI review requested due to automatic review settings June 23, 2025 21:24
@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Jun 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR standardizes type hints, formatting, and documentation across the Azure Storage Blob async and sync clients, and removes inline overloads from implementation files. Key changes include:

  • Added and updated .pyi stubs for async lease, container, and service clients.
  • Refactored BlobLeaseClient implementation for consistent quoting and parentheses placement.
  • Enhanced docstrings and type annotations in _blob_service_client_async.py and its sync counterpart; removed runtime @overload definitions from client implementations.

Reviewed Changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated no comments.

Show a summary per file
File Description
aio/_lease_async.pyi New async lease client stub
aio/_lease_async.py Cleaned class signature, uniform quoting, parentheses placement
aio/_container_client_async.pyi New async container client stub
aio/_blob_service_client_async.pyi New async service client stub
aio/_blob_service_client_async.py Added type hints to private methods, expanded docstrings
aio/_blob_client_async.py Adjusted imports to shared base client, removed runtime overloads
aio/init.py Simplified upload_blob_to_url call by removing redundant cast
_lease.pyi & _lease.py Aligned sync lease client with async updates
_container_client.pyi New sync container client stub
_blob_service_client.pyi New sync service client stub
_blob_service_client.py Added type hints, expanded docstrings
_blob_client_async.py & _blob_client.py Removed inline overloads, adjusted typing imports
init.py Simplified upload_blob_to_url call in sync entry point
Comments suppressed due to low confidence (2)

sdk/storage/azure-storage-blob/azure/storage/blob/aio/_blob_service_client_async.py:193

  • The from_connection_string docstring now lists many :keyword entries without corresponding :type or :param directives. For consistency with the Azure SDK Python Design Guidelines, consider adding :type api_version: str (and similarly for other :keyword params) or switch to using :param <name>: <description> + :type <name>: <type> blocks.
        :keyword str api_version:

sdk/storage/azure-storage-blob/azure/storage/blob/_blob_service_client.py:185

  • Similar to the async version, the sync from_connection_string docstring uses only :keyword entries. Align with Sphinx conventions by pairing each :keyword with a :type directive or switching to :param/ :type style as per the Azure SDK Python Design Guidelines.
        :keyword str api_version:

Copy link

github-actions bot commented Jun 23, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure-storage-blob

AsyncStorageAccountHostsMixin,
StorageAccountHostsMixin,
StorageEncryptionMixin,
AbstractAsyncContextManager
Copy link
Member

Choose a reason for hiding this comment

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

I haven't fully reviewed this PR but echo-ing Jacobs comments here:
As Jacob mentioned, he proposed some possible changes but once you land on a solution, if you could get this consistent (i.e. if AbstractAsyncContextManager needs to stay, there should be a sync equivalent).

Copy link
Member Author

Choose a reason for hiding this comment

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

Think the solution here to annotate classmethods, __enter__s, and __aenter__s return type as Self. For async only, we have to add AbstractAsyncContextManager. This way we prevent users from manually having to disable not-context-manager when working with client context managers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants