Skip to content

Rewrite az in python - #10516

Merged
Feng Zhou (fengzhou-msft) merged 7 commits into
Azure:devfrom
suominen:az-python
Nov 5, 2019
Merged

Feng Zhou (fengzhou-msft) merged 7 commits into
Azure:devfrom
suominen:az-python

Conversation

@suominen

@suominen Kimmo Suominen (suominen) commented Sep 12, 2019 •

Copy link
Copy Markdown
Contributor

Use python instead of bash, so that we get benefits from using #!/usr/bin/env python:

There doesn't seem to be a measurable startup delay impact as has been reported with using entry_points/console_script (#7469).


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

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

Use python instead of bash, so that we get benefits from of using
"#!/usr/bin/env python"

- distutils will replace it with an absolute path to the specific
  python interpreter being used (helps with tools like pipx)

- if running inside a virtualenv as is, it will run the python
  interpreter for that virtualenv (as found first in PATH)

There doesn't seem to be a measurable startup delay impact as has
been reported with using entry_points/console_script.
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Sep 12, 2019 •

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@fengzhou-msft Feng Zhou (fengzhou-msft) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code looks good to me, will do tests with python2 and python3

@suominen

Copy link
Copy Markdown
Contributor Author

I've tested it with

  • Python 2.7.16 and 3.7.3 on Debian 10.1 (buster).
  • Python 2.7.10 and 3.7.4 on macOS 10.14.6 (Mojave).

@suominen

Copy link
Copy Markdown
Contributor Author

Feng Zhou (@fengzhou-msft) - Didn't mean to dismiss any reviews, but just keep this branch without merge conflicts. Maybe it is better I don't resolve conflicts going forward -- it is just about HISTORY.rst changing in dev.

@yonzhan

Copy link
Copy Markdown
Collaborator

Feng Zhou (@fengzhou-msft) please help deal with this PR.

@fengzhou-msft

Copy link
Copy Markdown
Member

Yong Zhang (@yonzhan) I have tested the code, it works as stated and solved several issues. This change impacts the entry point for running all commands when installed with pip. We need Zim Kalinowski (@zikalino) to review and see if there's any drawbacks.

@yonzhan

Copy link
Copy Markdown
Collaborator

Zim Kalinowski (@zikalino) please review this PR.

@fengzhou-msft

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Pull request contains merge conflicts.

@fengzhou-msft

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

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.

4 participants