Skip to content
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

[Storage] Fix #21414: az storage blob sync: Fix the flag --delete-destination default to false #21662

Merged
merged 12 commits into from
Apr 18, 2022

Conversation

jonie001
Copy link
Contributor

…ation default to false

Description

Fix #21414

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 feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Mar 16, 2022
@ghost
Copy link

ghost commented Mar 16, 2022

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

@ghost ghost added the Auto-Assign Auto assign by bot label Mar 16, 2022
@ghost ghost requested a review from yonzhan March 16, 2022 08:39
@ghost ghost assigned evelyn-ys Mar 16, 2022
@ghost ghost added the Storage az storage label Mar 16, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Mar 16, 2022

Storage bug fix

@yonzhan yonzhan added this to the Apr 2022 (2022-05-03) milestone Mar 16, 2022
@jonie001 jonie001 changed the title [Storage] Fix#21414:az storage blob sync:Fix the flag --delete-destination default to false [Storage] Fix#21414: az storage blob sync: Fix the flag --delete-destination default to false Mar 16, 2022
@evelyn-ys
Copy link
Member

Changing the default value is breaking change. How about add a parameter --delete-destination which defaults to True? For customers complaining about this behavior, they can customize to Flase by themselves.

@@ -1256,6 +1259,7 @@ def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statem
c.argument('source', options_list=['--source', '-s'],
help='The source file path to sync from.')
c.ignore('destination')
c.argument('delete_destination', delete_destination_type)
Copy link
Member

Choose a reason for hiding this comment

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

Try to use get_three_state_flag arg type

@@ -81,10 +81,14 @@ def storage_fs_directory_copy(cmd, source, destination, recursive=None, **kwargs
azcopy.copy(source, destination, flags=flags)


def storage_blob_sync(cmd, client, source, destination, exclude_pattern=None, include_pattern=None,
exclude_path=None):
def storage_blob_sync(cmd, client, source, destination, delete_destination=None, exclude_pattern=None,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def storage_blob_sync(cmd, client, source, destination, delete_destination=None, exclude_pattern=None,
def storage_blob_sync(cmd, client, source, destination, delete_destination=True, exclude_pattern=None,

In this way, default=True will automatically show in help message

@evelyn-ys evelyn-ys merged commit 96bbce4 into Azure:dev Apr 18, 2022
@evelyn-ys evelyn-ys changed the title [Storage] Fix#21414: az storage blob sync: Fix the flag --delete-destination default to false [Storage] Fix #21414: az storage blob sync: Fix the flag --delete-destination default to false Apr 18, 2022
@jonie001 jonie001 deleted the fix_storage_blob_sync branch May 27, 2022 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage az storage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

az storage blob sync's deletion behavior can lead to unexpected data loss
3 participants