-
Notifications
You must be signed in to change notification settings - Fork 124
Pre-release: extract metadata generation logic and decouple from wheel 0.30.0 #521
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
base: dev
Are you sure you want to change the base?
Conversation
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.
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- HISTORY.rst: Language not supported
| Release History | ||
| =============== | ||
| 0.2.3b1 | ||
| +++++++ |
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.
|
|
||
| 0.2.0 | ||
| +++++ | ||
| ++++++ |
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.
same as above
2d20ed2 to
49c5280
Compare
| metadata[low_key] = pkg_info[key] | ||
|
|
||
| metadata['metadata_version'] = METADATA_VERSION | ||
|
|
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.
It should be as what pkg_info.metadata_version is? like 2.2 now?
| pkg_info = read_pkg_info(path) | ||
| except Exception: | ||
| with open(path, 'rb') as pkg_info_file: | ||
| pkg_info = email.parser.Parser().parsestr(pkg_info_file.read().decode('utf-8')) |
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.
Another more straightforward way to get pkgInfo is pkginfo.Wheel for dist-info or pkginfo.Develop for egg-info, as az extension list does in azure cli core here: https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/azure/cli/core/extension/__init__.py#L154-L157
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 don't think it is a good idea to vendor code from wheel library. The code itself may become difficult to maintain.
Besides using pkginfo.Wheel as mentioned by #521 (comment), we may consider decoupling from wheel:
Solution 1: Define our own metadata in azext_metadata.json and add necessary fields, such as
{
"azext.isPreview": false,
"azext.minCliCoreVersion": "2.45.0"
"azext.name": "ssh"
"azext.description": "SSH into Azure VMs using RBAC and AAD OpenSSH Certificates"
"azext.long_description": "SSH into Azure VMs using RBAC and AAD OpenSSH Certificates. The client generates ..."
...
}
The content will be merged into metadata automatically:
azure-cli-dev-tools/azdev/operations/extensions/util.py
Lines 53 to 55 in 78af72c
| azext_metadata = _get_azext_metadata(ext_dir) | |
| if azext_metadata: | |
| metadata.update(azext_metadata) |
"metadata": {
"azext.isPreview": false,
"azext.minCliCoreVersion": "2.45.0",
"classifiers": [
"Development Status :: 4 - Beta",Solution 2: Use regex to extract necessary information directly from setup.py and plug it into a template to generate index.json entries.
This way, we will no longer be affected by wheel's changes.

Azure/azure-cli-extensions#7740