Skip to content

Conversation

@mlschodkowski
Copy link
Contributor

@mlschodkowski mlschodkowski commented Dec 11, 2024

Description

StorageStreamDownloader.read() on blob behaves differently when using chars vs size, but this should behave the same (according to documentation). This single change fixes discrepancies in number of chars read when specifying negative number in StorageStreamDownloader.read()

Example
download_blob().read(size=-1) behaves differently than download_blob(encoding='utf-8').read(chars=-1)

>>> upload("123456", ...)
>>> download_blob().read(size=-1)
# will return "123456" in bytes
>>> download_blob(encoding='utf-8').read(chars=-1)
"12345" # will return len(content) - 1 characters instead of full content

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

read() on blob behaves differently when using chars vs size, but this should behave the same (accoring to documentation).

This commit fixes discrepancies in number of chars read when specifying negative number in `StorageStreamDownloader.read()`
@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files) labels Dec 11, 2024
@github-actions
Copy link

Thank you for your contribution @777moneymaker! We will review the pull request and get back to you soon.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Add None value handling in chars
simplify None and int handling
Remove redundant code
Add handling of chars=0
@mlschodkowski mlschodkowski marked this pull request as draft December 19, 2024 13:18
@mlschodkowski mlschodkowski marked this pull request as ready for review December 19, 2024 13:19
Copy link
Member

@jalauzon-msft jalauzon-msft left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I have been away for the holiday season. Thanks for your contribution! This change looks good, but could you please apply the same change to our async implementation here:

This can be tested using our async clients. Once that's done, we can approve and merge his change, thanks.

@mlschodkowski
Copy link
Contributor Author

Sorry for the delay, I have been away for the holiday season. Thanks for your contribution! This change looks good, but could you please apply the same change to our async implementation here:

This can be tested using our async clients. Once that's done, we can approve and merge his change, thanks.

Hey, no problem. The change has just been committed.

@mlschodkowski
Copy link
Contributor Author

@microsoft-github-policy-service agree

@jalauzon-msft jalauzon-msft merged commit 174f2fd into Azure:main Jan 6, 2025
21 checks passed
@jalauzon-msft
Copy link
Member

Hi @777moneymaker Leon, thanks again for your contribution! I have approved and merged this and it will be included in our next release.

l0lawrence pushed a commit to l0lawrence/azure-sdk-for-python that referenced this pull request Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants