Performance: Stop using pkg_resources/declare_namespace #2951
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing pylint on azure-cli-nspkg namespace package
…from check for valid license header
…er (which obviously can't be found)
…same azure_bdist_wheel.py file that the azure sdk is using
… file as the rest of the azure sdk. This required adding an azure-cli-command_modules-nspkg package since the sdk version uses the name of the package as the path in which to look for/exclude __init__.py files.
…i-command-module-nspkg since that makes setuptools happier.
lmazuel
suggested changes
Apr 24, 2017
| @@ -47,7 +47,7 @@ def build_package(path_to_package, dist_dir): | |||
| print_heading('Building {}'.format(path_to_package)) | |||
| path_to_setup = os.path.join(path_to_package, 'setup.py') | |||
| set_version(path_to_setup) | |||
| cmd_success = exec_command('python setup.py sdist -d {0} bdist_wheel -d {0}'.format(dist_dir), cwd=path_to_package) | |||
| cmd_success = exec_command('python setup.py bdist_wheel -d {0}'.format(dist_dir), cwd=path_to_package) | |||
There was a problem hiding this comment.
You can still do both if you need
| zip_safe=False, | ||
| classifiers=CLASSIFIERS, | ||
| install_requires=[ | ||
| 'azure-cli-nspkg>=1.0.0' |
| 'azure.cli.core', | ||
| 'azure.cli.core.commands', | ||
| 'azure.cli.core.extensions', | ||
| 'azure.cli.core.sdk', | ||
| 'azure.cli.core.profiles', | ||
| ], | ||
| install_requires=DEPENDENCIES | ||
| install_requires=DEPENDENCIES, |
There was a problem hiding this comment.
I can't comment upper, but if you want setuptools to work out of the box, you should remove "azure-cli-nspkg" as a direct dependencies of the package. This way the "setuptools" install does not install it, and azure_bdist_wheel will add it automatically to the wheel.
There was a problem hiding this comment.
Yep - thanks, missed that. Fixed!
- set correct version requirement for azure-cli-nspkg in the command_modules nspkg setup.py - Remove hard-coded dependency on azure cli nspkg from azure cli core and let the bdist_wheel extension added it required.
Codecov Report
@@ Coverage Diff @@
## master #2951 +/- ##
==========================================
+ Coverage 63.33% 63.39% +0.06%
==========================================
Files 466 463 -3
Lines 26539 26579 +40
Branches 4069 4081 +12
==========================================
+ Hits 16809 16851 +42
+ Misses 8617 8612 -5
- Partials 1113 1116 +3
Continue to review full report at Codecov.
|
lmazuel
approved these changes
Apr 25, 2017
|
@johanste LGTM but didn't clone the branch. If you want I can clone it tomorrow and build to check everything is fine |
|
@lmazuel, we would very much appreciate you cloning & building. We do intend to merge today. |
This was referenced May 13, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Loading pkg_resources and calling declare_namespace caused startup performance to suffer. This change adopts the solution from the azure sdk, where pkg_resources are only used for dev/editable installs and are stripped out when building wheels.
In order to be able to reuse the azure_bdist_wheel.py wheel extension implementation from the azure sdk, we also introduced an additional namespace package (the name of the namespace package is used to determine which init.py files to exclude when building the wheel)
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Command Guidelines
(see Authoring Command Modules)