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 #27590: az storage fs directory download: Check user sytem PATH for azcopy and use CLI config directory for new install #27593

Merged
merged 8 commits into from Oct 19, 2023

Conversation

calvinhzy
Copy link
Member

@calvinhzy calvinhzy commented Oct 13, 2023

Related command

Description

Fix #27590 Check user sytem PATH for azcopy and use it if the version is newer than 10.13.0. If not found or has old version, use CLI config directory for azcopy

Testing Guide

History Notes

[Storage] Fix #27590: az storage fs directory download: Check user sytem PATH for azcopy and use CLI config directory for new install


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

@calvinhzy calvinhzy self-assigned this Oct 13, 2023
@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Oct 13, 2023

🔄AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.11
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.9
🔄mysql
🔄latest
🔄3.11
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Oct 13, 2023

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Oct 13, 2023

Storage

@jakevc
Copy link

jakevc commented Oct 16, 2023

Some more thoughts here.

The desired behavior is for az CLI to use azcopy binary for some sub commands. The issue was that the az CLI command would download azcopy binary to an install location that is common for users to install software ~/bin, replacing the binary that may already exist at that path.

The proposed solution here is to use the azcopy binary that already exists if one can be found.. but that might not be ideal either because az CLI might depend on features of a specific azcopy version (right now it's pinned at 10.13, but the latest version is 10.21.0), causing more people to file issues about the az storage CLI....

A more stable solution that doesn't break the users local version of azcopy, or potentially break az cli commands by using an incompatible version of azcopy, would be to have a more unique default install location for the az cli's pinned version of azcopy.

For example, suppose I have a local version of azcopy: ~/bin/azcopy, who's --version is 10.21.0. Then I run az storage command and that installs the compatible version of azcopy in it's own unique install location, say ~/.azure/cliextensions/storage-preview/bin/azcopy. Then az cli can have it's own pinned version of azcopy, and I can have my own pinned version of azcopy, and everyone is happily unclobbered.

@calvinhzy
Copy link
Member Author

@jakevc Having a pinned version in another location is probably a better solution than what I was trying to do here. Will keep the 10.13.0 as the version.

@calvinhzy calvinhzy marked this pull request as ready for review October 17, 2023 07:42
@evelyn-ys
Copy link
Member

I do think what jake said is a good suggestion. Can we detect whether there's existing azcopy installed before CLI downloads and installs specific azcopy? There's no necessary for CLI to pin azcopy's version. Ideally CLI can smoothly call any version installed by customer as CLI just pass through arguments to azcopy

@evelyn-ys
Copy link
Member

BTW, resource module installs bicep under our config directory, shall we follow the same path?

_config_dir = get_config_dir()
_bicep_installation_dir = os.path.join(_config_dir, "bin")

@calvinhzy calvinhzy changed the title [Storage] Fix #27590: az storage fs directory download: Check system path for azcopy before installing [Storage] Fix #27590: az storage fs directory download: Make a more unique directory for azcopy used by CLI Oct 18, 2023
…er than 10.13.0, use the cli config dir following the same convention as bicep
@calvinhzy calvinhzy changed the title [Storage] Fix #27590: az storage fs directory download: Make a more unique directory for azcopy used by CLI [Storage] Fix #27590: az storage fs directory download: Check user sytem PATH for azcopy, use CLI config directory for azcopy Oct 19, 2023
@calvinhzy calvinhzy changed the title [Storage] Fix #27590: az storage fs directory download: Check user sytem PATH for azcopy, use CLI config directory for azcopy [Storage] Fix #27590: az storage fs directory download: Check user sytem PATH for azcopy and use CLI config directory for new install Oct 19, 2023
@calvinhzy calvinhzy merged commit f1d068e into Azure:dev Oct 19, 2023
54 checks passed
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 Storage az storage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

az storage fs directory download replaces local azcopy binary with old version
4 participants