[Storage] az storage fs file download: Stream ADLS Gen2 downloads to avoid content-encoding decode failures - #33730
[Storage] az storage fs file download: Stream ADLS Gen2 downloads to avoid content-encoding decode failures#33730Aditya Pujara (a0x1ab) with Copilot wants to merge 7 commits into
az storage fs file download: Stream ADLS Gen2 downloads to avoid content-encoding decode failures#33730Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
az storage fs file download: Stream ADLS Gen2 downloads to avoid content-encoding decode failures
Live test results —
|
There was a problem hiding this comment.
✅ Review: All checks passed
Live test: Run #29383646373 — success
CI checks: 1 passed, 0 failed, 0 pending
Summary
PR #33730 fixes az storage fs file download failures on ADLS Gen2 files with Content-Encoding: deflate by switching from buffered readall() to streamed readinto(stream). This aligns with the existing stream-to-file pattern used elsewhere in storage downloads.
All checks are green and the live test completed successfully. This PR is ready for maintainer review and merge.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Storage |
There was a problem hiding this comment.
❌ CI Failures
The following CI checks failed on this PR:
Azure.azure-cli— the main CI pipeline failedAzure.azure-cli (Test Homebrew Formula)— the Homebrew formula test failed
Please investigate the failures in the CI run logs and fix the issues, then push a new commit.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Copilot the automated CI and live-test review on this PR found failures. Please look into the requested changes here: #33730 (review) and push a fix. Posted by agent-assist (autonomous bug-fix pipeline). |
Pushed |
There was a problem hiding this comment.
Automated Review — Changes Requested
PR: [Storage] az storage fs file download: Stream ADLS Gen2 downloads to avoid content-encoding decode failures (#33730)
Live test (Tester)
- Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/29709280743 — failed
- Job
azdev test --live (PR 33730)failed. Please check the live test output for theaz storage fs file downloadcommand and fix the underlying issue (log fetch was unavailable due to a transient GitHub API outage, so please re-check the run directly).
CI status (azure-cli, head SHA de4bbe4)
3 checks failed, 23 still pending/in progress at the time of this review:
- Test Homebrew Formula — failed: https://github.com/Azure/azure-cli/runs/88252678005
- Integration Test against Profiles Python312 — failed: https://github.com/Azure/azure-cli/runs/88251764895
- Build Windows MSI x64 — failed: https://github.com/Azure/azure-cli/runs/88251561172
Please investigate these failures (Homebrew formula validation, the Python 3.12 profile integration tests, and the Windows MSI build), along with the live-test failure above, and push a fix. Once the changes are in, CI and the live test will automatically re-run.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
This PR updates Azure Storage file downloads to stream content directly to disk (avoiding full in-memory reads) and hardens Homebrew-based CI packaging steps when azure-cli isn’t preinstalled.
Changes:
- Switch
download_filefromreadall()+write()to streaming viareadinto(stream). - Add a unit test that verifies
readintois used (andreadallis not). - Make Homebrew uninstall steps conditional to avoid failing when
azure-cliisn’t installed.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/storage/operations/fs_file.py | Streams file downloads to destination using readinto instead of buffering whole content. |
| src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_fs_file_operations.py | Adds coverage ensuring streaming behavior (readinto) is used. |
| scripts/release/homebrew/test_homebrew_package.sh | Avoids failing on brew uninstall azure-cli when the formula isn’t installed. |
| azure-pipelines.yml | Mirrors the conditional Homebrew uninstall logic in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| download = mock.Mock() | ||
|
|
||
| def _write_to_stream(target_stream): | ||
| return target_stream.write(b'hello world') | ||
|
|
||
| download.readinto.side_effect = _write_to_stream |
| if brew list --versions azure-cli >/dev/null 2>&1; then | ||
| brew uninstall azure-cli | ||
| else | ||
| echo "azure-cli is not pre-installed" | ||
| fi |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
🔔 Routing this PR to @Azure/act-codegen-extensibility-squad. |
Live test results —
|
There was a problem hiding this comment.
❌ CI Failure
Check failed: Azure.azure-cli (Test Homebrew Formula) — https://github.com/Azure/azure-cli/runs/88252678005
The Homebrew Formula test job failed on the latest commit for this PR. Please investigate the failure log and push a fix. All other checks are currently passing.
|
Started a Copilot task using |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Live test results —
|
Live test results —
|
There was a problem hiding this comment.
CI Failure Summary
The Azure DevOps validation pipeline Azure.azure-cli failed on the current head commit 3fd5b854e7cc64d0f607f588d574b714002ae2e6:
- Check:
Azure.azure-cli— Build #20260819.30 failed (4 errors / 0 warnings) - Build results: https://dev.azure.com/azclitools/5147fa83-336e-44ef-bbe0-c86b8ae86cbb/_build/results?buildId=339733
All other checks passed (104/105) and the live test run for this PR previously succeeded, so the regression appears isolated to this build. Please open the build results link above, review the 4 reported errors (likely lint/style or unit-test failures introduced by the ADLS Gen2 streaming download change in az storage fs file download), and push a fix.
|
Started a Copilot task using |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Live test results —
|
🤖 PR Validation — ️✔️ All clear
Related command
az storage fs file downloadDescription
az storage fs file downloadcan fail on ADLS Gen2 files when the response carriesContent-Encoding: deflate; upload/list/show paths succeed, but download trips the SDKreadall()decode path.readall()to streamedreadinto(stream).storage fs file downloadwith the existing stream-to-file pattern used elsewhere in storage download flows.readinto()and does not depend onreadall().Testing Guide
Example reproduction path:
Expected: file is written successfully even when the service response includes
Content-Encoding.History Notes
[Storage]
az storage fs file download: Stream ADLS Gen2 downloads to avoid content-encoding decode failuresThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.