upload: add attestations to PackageFile #1098
Conversation
Signed-off-by: William Woodruff <[email protected]>
| @@ -186,6 +188,9 @@ def metadata_dictionary(self) -> Dict[str, MetadataValue]: | |||
| if self.gpg_signature is not None: | |||
| data["gpg_signature"] = self.gpg_signature | |||
|
|
|||
| if self.attestations is not None: | |||
| data["attestations"] = json.dumps(self.attestations) | |||
There was a problem hiding this comment.
I'm 99% certain I remember that this ends up in a multipart/form-data body. Does PyPI want this to have a content type? Also, does this need to be conditioned on index server? I don't expect any of the third party ones to support this any time soon
There was a problem hiding this comment.
In the current draft, we expect it to be in that multipart/form-data body: https://peps.python.org/pep-0740/#upload-endpoint-changes. No content-type marker is expected.
(I have no objection to changing this to require a content-type like the main content field, if you think that would be clearer! I'd have to tweak the PEP in that case.)
Also, does this need to be conditioned on index server? I don't expect any of the third party ones to support this any time soon
Everything is currently flagged behind --attestations which is off by default, so in principle this shouldn't need to be conditioned on the index server. That being said I could additionally add that check, although I think third party indices/mirrors may eventually want to support these as well and may be surprised by twine pre-filtering them 🙂
There was a problem hiding this comment.
Yeah, I don't know. I'm more worried about people enabling this against a not-PyPI index that won't ignore the field and complaining about the errors/failures to upload
There was a problem hiding this comment.
I'm more worried about people enabling this against a not-PyPI index that won't ignore the field and complaining about the errors/failures to upload
Ah yeah, hadn't thought of that. Support on non-PyPI indices is a distant idea anyways so I'll flag this off 🙂
There was a problem hiding this comment.
We can always add another option to enable it on a per-index basis later. But yeah, I just dread every 3rd party index issue we get because it is always a nightmare of the index doing something that violates a specification
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
|
Should be good for another look! I've rounded out the coverage as well. |
WIP, needs more tests.Summary of changes:
_make_packageto accept anattestationsparameter, which receives the list of attestations for the distribution (from_split_inputs).--attestationsis set andattestationsis nonempty, the supplied attestations are passed into the constructedPackageFile.PackageFilereads each attestation as JSON and compounds it onto a JSON array to send as theattestationsfield during upload, per PEP 740.See #1094.