publish: use the artifact metadata instead of the project metadata #10624
+142
−52
Conversation
This file contains hidden or 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
Reviewer's GuideThe PR refactors the uploader to source package metadata directly from the distribution artifacts (wheel or sdist) instead of the project model, simplifying and unifying the upload payload construction. Class diagram for updated Uploader metadata handlingclassDiagram
class Uploader {
+upload(session, url, dry_run, skip_existing)
+post_data(file: Path) dict[str, Any]
+_upload_file(file: Path)
+_prepare_data(data: dict[str, Any]) list[tuple[str, str]]
+_get_type(file: Path) Literal["bdist_wheel", "sdist"]
+_get_metadata(file: Path) RawMetadata
+_is_file_exists_error(response: requests.Response) bool
-_package
}
Uploader ..> HashManager
Uploader ..> RawMetadata
Uploader ..> Path
Uploader ..> requests.Response
Flow diagram for artifact metadata extraction in upload processflowchart TD
A["Start upload"] --> B["Select file (wheel or sdist)"]
B --> C["Determine file type"]
C --> D["Extract metadata from artifact"]
D --> E["Construct upload payload from artifact metadata"]
E --> F["Send payload to upload API"]
F --> G["Finish upload"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/poetry/publishing/uploader.py:139-155` </location>
<code_context>
- # Metadata 2.1
- if meta.description_content_type:
- data["description_content_type"] = meta.description_content_type
+ for key, value in cls._get_metadata(file).items():
+ # strip trailing 's' to match API field names
+ # see https://docs.pypi.org/api/upload/
+ if key in {"platforms", "supported_platforms", "license_files"}:
+ key = key[:-1]
- # TODO: Provides extra
+ # revert some special cases from packaging.metadata.parse_email()
+
+ # "keywords" is not "multiple use" but a comma-separated string
+ if key == "keywords":
+ value = ", ".join(value)
+
+ # "project_urls" is not a dict
+ if key == "project_urls":
+ value = [f"{k}, {v}" for k, v in value.items()]
+
+ data[key] = value
return data
</code_context>
<issue_to_address>
**suggestion:** Stripping trailing 's' from metadata keys may cause mismatches with expected API fields.
Centralize the mapping of plural to singular keys or use explicit key handling to prevent future mismatches if API field names change or new plural keys are added.
```suggestion
# Centralized mapping for plural-to-singular metadata keys
PLURAL_TO_SINGULAR_KEYS = {
"platforms": "platform",
"supported_platforms": "supported_platform",
"license_files": "license_file",
}
for key, value in cls._get_metadata(file).items():
# Use explicit mapping for plural keys
if key in PLURAL_TO_SINGULAR_KEYS:
key = PLURAL_TO_SINGULAR_KEYS[key]
# revert some special cases from packaging.metadata.parse_email()
# "keywords" is not "multiple use" but a comma-separated string
if key == "keywords":
value = ", ".join(value)
# "project_urls" is not a dict
if key == "project_urls":
value = [f"{k}, {v}" for k, v in value.items()]
data[key] = value
```
</issue_to_address>
### Comment 2
<location> `src/poetry/publishing/uploader.py:148-149` </location>
<code_context>
+ # revert some special cases from packaging.metadata.parse_email()
+
+ # "keywords" is not "multiple use" but a comma-separated string
+ if key == "keywords":
+ value = ", ".join(value)
+
+ # "project_urls" is not a dict
</code_context>
<issue_to_address>
**nitpick:** Joining keywords with a comma and space may not match all consumers' expectations.
Verify that the API documentation specifies this format for keywords, or adjust to use only a comma if needed.
</issue_to_address>
### Comment 3
<location> `src/poetry/publishing/uploader.py:152-153` </location>
<code_context>
+ value = ", ".join(value)
+
+ # "project_urls" is not a dict
+ if key == "project_urls":
+ value = [f"{k}, {v}" for k, v in value.items()]
+
+ data[key] = value
</code_context>
<issue_to_address>
**issue (bug_risk):** Formatting project_urls as a list of comma-separated strings may not match API expectations.
Verify the required format for project_urls in the upload API to ensure compatibility.
</issue_to_address>
### Comment 4
<location> `src/poetry/publishing/uploader.py:295` </location>
<code_context>
- def _get_type(self, file: Path) -> str:
+ @staticmethod
+ def _get_type(file: Path) -> Literal["bdist_wheel", "sdist"]:
exts = file.suffixes
if exts[-1] == ".whl":
</code_context>
<issue_to_address>
**issue:** Using file.suffixes without checking for empty lists may raise IndexError.
Accessing exts[-1] and exts[-2:] without checking if exts is non-empty can cause IndexError. Please add a check to handle cases where file.suffixes is empty.
</issue_to_address>
### Comment 5
<location> `src/poetry/publishing/uploader.py:305-314` </location>
<code_context>
+ def _get_metadata(file: Path) -> RawMetadata:
</code_context>
<issue_to_address>
**suggestion:** The _get_metadata method does not handle .zip sdists, which are valid in some cases.
Support for .zip sdists should be added for compatibility, or the limitation should be documented if intentional.
</issue_to_address>
### Comment 6
<location> `tests/publishing/test_uploader.py:162-171` </location>
<code_context>
+def test_uploader_post_data_wheel(fixture_dir: FixtureDirGetter) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for wheels with missing or malformed METADATA files.
Tests should also cover scenarios where the wheel lacks a METADATA file or has a malformed METADATA, verifying that Uploader.post_data raises the appropriate exceptions.
</issue_to_address>
### Comment 7
<location> `tests/publishing/test_uploader.py:203-200` </location>
<code_context>
+def test_uploader_post_data_sdist(fixture_dir: FixtureDirGetter) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for sdist files with missing or malformed PKG-INFO.
Please add tests for sdists missing PKG-INFO or with malformed PKG-INFO to ensure Uploader.post_data and _get_metadata handle these cases as expected.
Suggested implementation:
```python
def test_uploader_post_data_sdist(fixture_dir: FixtureDirGetter) -> None:
file = fixture_dir("simple_project") / "dist" / "simple_project-1.2.3.tar.gz"
assert Uploader.post_data(file) == {
"md5_digest": "e611cbb8f31258243d90f7681dfda68a",
"sha256_digest": "c4a72becabca29ec2a64bf8c820bbe204d2268f53e102501ea5605bc1c1675d1",
"blake2_256_digest": "d3df22f4944f6acd02105e7e2df61ef63c7b0f4337a12df549ebc2805a13c2be",
"filetype": "sdist",
"pyversion": "source",
"metadata_version": "2.1",
"name": "simple-project",
"version": "1.2.3",
}
import io
import tarfile
import tempfile
import pytest
def _make_sdist_without_pkg_info():
tmp = tempfile.NamedTemporaryFile(suffix=".tar.gz", delete=False)
with tarfile.open(tmp.name, "w:gz") as tar:
# Add a dummy file, but no PKG-INFO
info = tarfile.TarInfo(name="simple_project-1.2.3/dummy.txt")
data = b"dummy"
info.size = len(data)
tar.addfile(info, io.BytesIO(data))
return tmp.name
def _make_sdist_with_malformed_pkg_info():
tmp = tempfile.NamedTemporaryFile(suffix=".tar.gz", delete=False)
with tarfile.open(tmp.name, "w:gz") as tar:
# Add malformed PKG-INFO
info = tarfile.TarInfo(name="simple_project-1.2.3/PKG-INFO")
data = b"This is not a valid PKG-INFO"
info.size = len(data)
tar.addfile(info, io.BytesIO(data))
return tmp.name
def test_uploader_post_data_sdist_missing_pkg_info():
sdist_path = _make_sdist_without_pkg_info()
with pytest.raises(Exception):
Uploader.post_data(sdist_path)
def test_uploader_post_data_sdist_malformed_pkg_info():
sdist_path = _make_sdist_with_malformed_pkg_info()
with pytest.raises(Exception):
Uploader.post_data(sdist_path)
```
- If Uploader.post_data does not currently raise an Exception for these cases, you may need to adjust the assertion to check for None or partial data, or update Uploader to raise a specific error.
- You may want to use more specific exception types if Uploader.post_data raises them (e.g., ValueError).
- If you have a _get_metadata function exposed, you can add similar tests for it.
</issue_to_address>
### Comment 8
<location> `src/poetry/publishing/uploader.py:324-325` </location>
<code_context>
@staticmethod
def _get_metadata(file: Path) -> RawMetadata:
if file.suffix == ".whl":
with zipfile.ZipFile(file) as z:
for name in z.namelist():
parts = Path(name).parts
if (
len(parts) == 2
and parts[1] == "METADATA"
and parts[0].endswith(".dist-info")
):
with z.open(name) as mf:
return parse_email(mf.read().decode("utf-8"))[0]
raise FileNotFoundError("METADATA not found in wheel")
elif file.suffixes[-2:] == [".tar", ".gz"]:
with tarfile.open(file, "r:gz") as tar:
for member in tar.getmembers():
parts = Path(member.name).parts
if len(parts) == 2 and parts[1] == "PKG-INFO":
pf = tar.extractfile(member)
if pf:
return parse_email(pf.read().decode("utf-8"))[0]
raise FileNotFoundError("PKG-INFO not found in sdist")
raise ValueError(f"Unsupported file type: {file}")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if pf := tar.extractfile(member):
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
radoering
force-pushed
the
publish-artifact-metadata
branch
from
106dbf0 to
e102e6e
Compare
Secrus
approved these changes
Nov 20, 2025
Especially, since we do support other build backends than poetry-core this is more correct.
radoering
force-pushed
the
publish-artifact-metadata
branch
from
e102e6e to
c1b5f21
Compare
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Especially, since we do support other build backends than poetry-core this is more correct.
Pull Request Check List
This resolves an issue noticed in python-poetry/poetry-core#894 (comment)
Summary by Sourcery
Revise the package upload logic to derive metadata from distribution files (wheel and sdist) instead of the project, improving compatibility with alternative build backends.
New Features:
Bug Fixes:
Enhancements:
Tests: