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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
PYTHONPATHis not necessary anymore. By showing it withecho %PYTHONPATH%:This is not a valid path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also path separator
/is not valid on Windows.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix the path separator.
PYTHONPATH is not needed for normal cli run, but let's leave it there for the convenience of using additional custom Python libraries that users do not want to install in the global default location (i.e., the site-packages directory).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linux counterpart script also has this setting: https://github.com/Azure/azure-cli/blob/dev/src/azure-cli/az#L6..L14
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if there is a historical reason. As far as I can find, the only possible operation for
srcto exist is installing viapip install -e <VCS>:pip install -e "git+https://github.com/Azure/azure-sdk-for-python/#egg=azure-mgmt-eventgrid&subdirectory=sdk\eventgrid\azure-mgmt-eventgrid"Even in that case
srcshould be under%~dp0\... The path toazure-mgmt-eventgridis added tosys.pathviaenv\Lib\site-packages\azure-mgmt-eventgrid.egg-link.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree
%~dp0\..is a better location to putsrcas%~dp0is theScriptsdirectory (orbindirectory on Linux). I would consider changing it together with the Linux script when we have more knowledge about the real uses cases and history for thisPYTHONPATHsetting.