-
Notifications
You must be signed in to change notification settings - Fork 117
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
@@ -2,6 +2,10 @@ | |||
|
|||
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.
@@ -11,7 +15,7 @@ Release History | |||
* `azdev extension cal-next-version`: Adjust `minor` or `patch` update for previous preview versioning pattern. | |||
|
|||
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