{Storage} Remove DATA_STORAGE and DATA_COSMOS_TABLE client type reference#31583
Merged
Conversation
️✔️AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| storage blob incremental-copy cancel | cmd storage blob incremental-copy cancel added parameter account_url |
||
| storage blob incremental-copy cancel | cmd storage blob incremental-copy cancel update parameter lease_id: updated property name from lease_id to lease |
||
| storage blob incremental-copy start | cmd storage blob incremental-copy start added parameter account_url |
||
| storage blob incremental-copy start | cmd storage blob incremental-copy start update parameter destination_if_modified_since: removed property type=custom_type |
||
| storage blob incremental-copy start | cmd storage blob incremental-copy start update parameter destination_if_unmodified_since: removed property type=custom_type |
||
| storage blob incremental-copy start | cmd storage blob incremental-copy start update parameter destination_lease_id: updated property name from destination_lease_id to destination_lease |
||
| storage blob incremental-copy start | cmd storage blob incremental-copy start update parameter source_lease_id: updated property name from source_lease_id to source_lease |
||
| storage blob sync | cmd storage blob sync added parameter account_url |
||
| storage blob sync | cmd storage blob sync added parameter extra_options |
||
| storage blob sync | cmd storage blob sync update parameter extra_options: removed property nargs=* |
||
| storage blob sync | cmd storage blob sync update parameter extra_options: updated property name from extra_options to account_name |
||
| storage blob sync | cmd storage blob sync update parameter extra_options: updated property options from [] to ['--account-name'] |
||
| storage copy | cmd storage copy added parameter account_url |
||
| storage copy | cmd storage copy added parameter extra_options |
||
| storage copy | cmd storage copy update parameter extra_options: added property options_deprecate_info=[{'target': '--destination-account-name', 'redirect': '--account-name'}] |
||
| storage copy | cmd storage copy update parameter extra_options: removed property nargs=* |
||
| storage copy | cmd storage copy update parameter extra_options: updated property name from extra_options to account_name |
||
| storage copy | cmd storage copy update parameter extra_options: updated property options from [] to ['--account-name', '--destination-account-name'] |
||
| storage cors add | cmd storage cors add added parameter account_url |
||
| storage cors clear | cmd storage cors clear added parameter account_url |
||
| storage cors list | cmd storage cors list added parameter account_url |
||
| storage fs directory download | cmd storage fs directory download added parameter account_url |
||
| storage fs directory upload | cmd storage fs directory upload added parameter account_url |
||
| storage logging off | cmd storage logging off added parameter account_url |
||
| storage logging show | cmd storage logging show added parameter account_url |
||
| storage logging update | cmd storage logging update added parameter account_url |
||
| storage metrics show | cmd storage metrics show added parameter account_url |
||
| storage metrics update | cmd storage metrics update added parameter account_url |
||
| storage remove | cmd storage remove added parameter account_url |
Collaborator
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
…tests not fully passing
calvinhzy
changed the title
… logs`, `az acr task run`, `az acr task logs`, `az acr pack build`
…batchai cluster file list`
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
calvinhzy
changed the title
calvinhzy
requested review from
bebound,
jiasli,
jsntcy,
kairu-ms,
necusjz,
wangzelin007,
yanzhudd and
zhoxing-ms
as code owners
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes deprecated DATA_STORAGE and DATA_COSMOS_TABLE client type references from the Azure CLI storage module, upgrading to more modern storage SDKs. The changes focus on modernizing the storage command module by removing legacy Track1 SDK dependencies and consolidating on Track2 implementations.
- Updates dependency versions for
azure-multiapi-storagefrom 1.4.1 to 1.5.0 across all platform requirement files - Removes deprecated DATA_STORAGE and DATA_COSMOS_TABLE resource type usage in favor of more specific resource types
- Implements incremental blob copy operations using AAZ (Azure AutoRest for CLI) framework instead of legacy SDKs
Reviewed Changes
Copilot reviewed 41 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py & requirements files | Updates azure-multiapi-storage dependency from 1.4.1 to 1.5.0 |
| vm/custom.py | Modernizes blob client usage from BlockBlobService to BlobClient |
| storage/util.py | Removes deprecated Track1 SDK utility functions |
| storage/tests | Updates test expectations and removes deprecated API version constraints |
| storage/operations | Adds new AAZ-based incremental copy implementation |
| storage/commands.py | Simplifies command registrations by removing deprecated client factories |
| storage/_validators.py | Updates validators to use Track2 SDK patterns exclusively |
Comments suppressed due to low confidence (1)
src/azure-cli/azure/cli/command_modules/vm/custom.py:48
- The condition
isinstance(blob.name, str)will always be true in Python 3, making the else clause unreachable. The original code checked forunicodetype which doesn't exist in Python 3. Consider removing this condition entirely or clarifying the intended logic.
from .aaz.latest.vm.disk import AttachDetachDataDisk
| self.assertEqual(actual, expected) | ||
| actual = str(services_type_v2()(input)) | ||
| expected = "bqtf" | ||
| self.assertEqual(set(actual), set(expected)) |
There was a problem hiding this comment.
The test is comparing sets instead of strings, which may not properly validate the exact output format. The original test compared strings directly. This change could mask ordering issues or allow incorrect character combinations.
Suggested change
| self.assertEqual(set(actual), set(expected)) | |
| self.assertEqual(actual, expected) |
|
|
||
| _Start.PageBlobCopyIncremental.on_202 = on_202 | ||
|
|
||
| _Start(command_args=cmd_args) |
There was a problem hiding this comment.
The function creates an instance _Start but doesn't handle potential exceptions that could occur during execution. Consider adding appropriate error handling around this call.
Suggested change
| _Start(command_args=cmd_args) | |
| try: | |
| _Start(command_args=cmd_args) | |
| except (HttpResponseError, AzureResponseError) as ex: | |
| logger.error("Failed to start incremental copy: %s", ex) | |
| raise CLIError(f"Failed to start incremental copy: {ex}") | |
| except Exception as ex: | |
| logger.error("An unexpected error occurred during incremental copy: %s", ex) | |
| raise CLIError(f"An unexpected error occurred during incremental copy: {ex}") |
Comment on lines
+72
to
75
| if not sas_token and client.credential and hasattr(client.credential, "account_key"): | ||
| sas_token = _generate_sas_token(cmd, client.account_name, client.credential.account_key, service=service, | ||
| resource_types='co', | ||
| permissions='rdl') |
There was a problem hiding this comment.
The code uses hasattr to check for account_key which is fragile. Consider using a more explicit type check or try-except pattern to handle different credential types more robustly.
Suggested change
| if not sas_token and client.credential and hasattr(client.credential, "account_key"): | |
| sas_token = _generate_sas_token(cmd, client.account_name, client.credential.account_key, service=service, | |
| resource_types='co', | |
| permissions='rdl') | |
| if not sas_token and client.credential: | |
| try: | |
| account_key = client.credential.account_key | |
| except AttributeError: | |
| account_key = None | |
| if account_key: | |
| sas_token = _generate_sas_token(cmd, client.account_name, account_key, service=service, | |
| resource_types='co', | |
| permissions='rdl') |
| ns['source_share'] = source_share | ||
| # get sas token for source | ||
| if not source_sas and not is_oauth: | ||
| if not source_sas and not is_oauth and source_key != 'fake_key': |
There was a problem hiding this comment.
The hardcoded string 'fake_key' comparison is a code smell. Consider using a constant or a more explicit way to identify test/fake credentials.
Suggested change
| if not source_sas and not is_oauth and source_key != 'fake_key': | |
| if not source_sas and not is_oauth and source_key != FAKE_STORAGE_ACCOUNT_KEY: |
evelyn-ys
approved these changes
Aug 7, 2025
zhoxing-ms
approved these changes
Aug 12, 2025
yanzhudd
approved these changes
Aug 12, 2025
wangzelin007
approved these changes
Aug 12, 2025
jsntcy
approved these changes
Aug 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related command
Description
Remove DATA_STORAGE and DATA_COSMOS_TABLE usages, which have been deprecated.
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis 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.