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] az storage copy: Add parameter --content-type to fix#6466 #12237

Merged
merged 6 commits into from
Mar 24, 2020

Conversation

Juliehzl
Copy link
Contributor

History Notes:
(Fill in the following template if multiple notes are needed, otherwise PR title will be used for history note.)

[Component Name 1] (BREAKING CHANGE:) (az command:) make some customer-facing change.
[Component Name 2] (BREAKING CHANGE:) (az command:) make some customer-facing change.


Fix #6466

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

@Juliehzl
Copy link
Contributor Author

There is an issue with azcopy 10.3 and the content-type cannot work as expected. Let's merge it when azcopy is ready.

@Juliehzl Juliehzl self-assigned this Feb 17, 2020
@@ -507,6 +507,8 @@ def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statem
c.argument('exclude_path', exclude_path_type)
c.argument('include_path', include_path_type)
c.argument('recursive', recursive_type)
c.argument('content_type', arg_group='Additional Flags', help="Specify content type of the file. "
"Implies no-guess-mime-type.")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use imperative mood for alignment? Also, "Implies" isn't very intuitive to me. Possibly "If unspecified, ..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. updated.

Copy link
Member

Choose a reason for hiding this comment

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

We misunderstood it! 😰 It means if --content-type is given, --no-guess-mime-type is implied.

@yonzhan yonzhan requested a review from qianwens March 19, 2020 02:16
@yonzhan yonzhan added this to the S167 milestone Mar 19, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Mar 19, 2020

add to S167

@Juliehzl Juliehzl requested review from zhoxing-ms and jiasli March 20, 2020 03:46
@@ -557,6 +557,8 @@ def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statem
c.argument('exclude_path', exclude_path_type)
c.argument('include_path', include_path_type)
c.argument('recursive', recursive_type)
c.argument('content_type', arg_group='Additional Flags', help="Specify content type of the file. "
"If unspecified, imply no-guess-mime-type.")
Copy link
Member

Choose a reason for hiding this comment

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

According to https://docs.microsoft.com/en-us/azure/storage/common/storage-ref-azcopy-copy#options

Suggested change
"If unspecified, imply no-guess-mime-type.")
"If unspecified, imply --no-guess-mime-type.")

@Juliehzl Juliehzl requested a review from jiasli March 24, 2020 04:03
Copy link
Member

@qianwens qianwens left a comment

Choose a reason for hiding this comment

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

:shipit:

@Juliehzl Juliehzl merged commit e830580 into Azure:dev Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow users to define content-type in batch upload
5 participants