Implement PEP 639, Metadata 2.4

[ewdurbin]

Closes #16620

Depends on merge/release of pypa/packaging#828
TODO: Move to release of packaging with license-expression parser

[miketheman]

Looking cool, and coming along nicely! I commented a bit inside.

One thing I'm curious about - at what point should we be looking to consolidate any file operations where we pop open the file and read stuff from it, so we only have to open/iterate once?

[ewdurbin]

One thing I'm curious about - at what point should we be looking to consolidate any file operations where we pop open the file and read stuff from it, so we only have to open/iterate once?

I thought about this a little and can imagine an approach of stashing a list of filenames when we iterate through to in _is_valid_dist_file, and fetching any files we actually need to read... at the moment that would only be .dist-info/METADATA. I wonder if a simple utility class wrapper that does this on __init__ would allow for us to make calls to get_wheel_metadata and contains or something would work... will poke a bit.

Edit: On second consideration... nearly all file uploads are dominated by either the advisory lock on journals or s3.putobject. Not sure this optimization is worth the effort at this point.

[ewdurbin]

@befeleme would you be able to take a look at this to review the implementation details?

[befeleme]

@befeleme would you be able to take a look at this to review the implementation details?

Yes, hopefully tomorrow.

[befeleme]

After reading the code I've got just one question: I can't locate the validation of the License-Expression per https://peps.python.org/pep-0639/#add-license-expression-field ("[Python Package Index (PyPI) MUST validate that they contain a valid, case-normalized license expression with valid identifiers and MUST reject uploads that do not"). Or did I miss it?

[ewdurbin]

After reading the code I've got just one question: I can't locate the validation of the License-Expression per https://peps.python.org/pep-0639/#add-license-expression-field ("[Python Package Index (PyPI) MUST validate that they contain a valid, case-normalized license expression with valid identifiers and MUST reject uploads that do not"). Or did I miss it?

That's implemented upstream in pypa/packaging, see pypa/packaging#828

[befeleme]

Ah, looking at the whole warehouse/forklift/metadata.py I see packaging validations are applied automatically and no additional changes regarding that are needed here. Makes sense.
I've got no other comments, it LGTM.

[miketheman]

Yay, we're almost there! I didn't re-review a lot of what I'd already covered, so I focused on the migration and testing, and looks like everything generally conforms to the PEP.

I'm thinking that tracking usage/progress of this can be deferred, but that might be a nice metric to emit via DB query later - I can imagine wanting to see new projects using the syntax, older projects converting to the new syntax, etc - but definitely beyond scope of this changeset.

Approving to you can decide when you want to run the migration, but please include some timeouts based on some guesswork from TestPyPI

[ewdurbin]

Re @miketheman:

I'm thinking that tracking usage/progress of this can be deferred, but that might be a nice metric to emit via DB query later - I can imagine wanting to see new projects using the syntax, older projects converting to the new syntax, etc - but definitely beyond scope of this changeset.

That is all technically captured in the immutable release metadata, so aside from watching a pot boil in real-time in datadog I agree adoption metrics are well beyond scope.

[ewdurbin]

Migrations are going out for this ahead of merge during this window to avoid unnecessary disruption: http://status.python.org/incidents/q5q5347n3s2b

[ofek]

Thanks everyone!