Skip to content

{Extension} insert extension directory in head of sys.path#12778

Closed
fengzhou-msft wants to merge 1 commit intoAzure:devfrom
fengzhou-msft:fix_wrong_sdk
Closed

{Extension} insert extension directory in head of sys.path#12778
fengzhou-msft wants to merge 1 commit intoAzure:devfrom
fengzhou-msft:fix_wrong_sdk

Conversation

@fengzhou-msft
Copy link
Member

Description of PR (Mandatory)
(Why this PR? What is changed? What is the effect? etc. A high-quality description can accelerate the review process)
Fix #12062.
The extension directory was appended to the sys.path, and the virtual env site-package directory was in front of it. So CLI will load the SDK installed in the virtual env instead of the one in extension directory. Now we put extension directory in front position of sys.path to be loaded priorly.

Testing Guide
(Example commands with explanations)

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.


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

@yonzhan
Copy link
Collaborator

yonzhan commented Mar 29, 2020

add to S167

@yungezz
Copy link
Member

yungezz commented Mar 29, 2020

hi @fengzhou-msft when user is using venv, it's by design all packages should be loaded from venv, right? Will sync with you offline on this.

Copy link
Contributor

@arrownj arrownj left a comment

Choose a reason for hiding this comment

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

LGTM

@jiasli
Copy link
Member

jiasli commented Mar 30, 2020

I debugged into


ext_dir is like 'C:\\Users\\xxx\\.azure\\cliextensions\\support'.

In that case, if two extensions contain the same SDK but with different versions, it will still has conflict.

Another concern is that allowing extensions to override SDKs from venv may cause a security breach - a malicious extension can inject tampered SDKs to intercept the calls thus overriding the behavior, also impacting other command modules that rely on the SDKs.

So as an ultimate solution, the extension should vendor the SDK by putting it under vendored_sdks to avoid the conflict. Then they can use something like from .vendored_sdks.timeseriesinsights.models import LongTermEnvironmentUpdateParameters.

@jiasli jiasli closed this Mar 30, 2020
@jiasli jiasli reopened this Mar 30, 2020
Copy link
Member

@jiasli jiasli left a comment

Choose a reason for hiding this comment

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

Better to sync with the issue creator to vendor the SDKs instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI calls the wrong version SDK when a different version SDK is installed in current pyenv

5 participants