Skip to content
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

Release 22.0 #569

Closed
pradyunsg opened this issue Jul 8, 2022 · 38 comments
Closed

Release 22.0 #569

pradyunsg opened this issue Jul 8, 2022 · 38 comments
Assignees

Comments

@pradyunsg
Copy link
Member

@pypa/packaging-committers Any concerns with me putting on the RM hat for making this release, and cutting it sometime in the next few weeks?

I think we can make this, once #484 merges.

/cc @sbidoul, to flag that I'd like this to be in time for pip 22.2. :)

@brettcannon
Copy link
Member

I'm good with doing a release. Did you want to be fancy and trying using sigstore to sign it?

@pradyunsg
Copy link
Member Author

I'm down to be fancy. :)

@brettcannon
Copy link
Member

The PyPA Discord has links to how urllib3 did it, so hopefully it's just something we can tie into the Nox release command and have it not be a big deal. Might actually simplify things if we choose to rely on it over GPG.

@pradyunsg
Copy link
Member Author

Posting links publicly, on a no-account-needed to view site, in case folks are interested:

@dstufft
Copy link
Member

dstufft commented Jul 16, 2022

FWIW, I was digging more into the metadata stuff, and I realized that the current (unreleased) metadata class is unable to represent otherwise valid metadata.

I'm working on a draft fix for it (among other things), but just wanted to raise the issue here.

@brettcannon
Copy link
Member

What specifically can it not represent? Is it the *-Email fields being too structured?

@ofek
Copy link
Contributor

ofek commented Sep 11, 2022

Any update on this?

@ofek
Copy link
Contributor

ofek commented Sep 12, 2022

Can I help in any way?

@brettcannon
Copy link
Member

@dstufft any clarifications you can give us?

@pradyunsg
Copy link
Member Author

@brettcannon @dstufft What do we want to do here?

Should we revert the metadata module, cut a release and then unrevert it?

Mark the metadata module as "experimental" and note that it might undergo unannounced API changes?

Hold off on the release until we resolve the unstated issue?

@pradyunsg
Copy link
Member Author

Outside of the potential metadata issue, we're basically good to go for a release.

@pradyunsg
Copy link
Member Author

Should we revert the metadata module, cut a release and then unrevert it?

If no one says anything by next week, I'm leaning toward this. :)

@brettcannon
Copy link
Member

If I can't get an answer from @dstufft then we can cut it out to make the release.

@brettcannon
Copy link
Member

From @dstufft privately:

The tl;dr is that the current metadata supports… RFC whatever for email addresses, which has some relatively obscure features for lists of email addresses.

I suggested just making the appropriate fields to strings.

Donald also said:

my branch also changed the API of the metadata class IIRC, for a few reasons, which i think we might want to do?

The big thing is I removed the default constructor for metadata, so that we can have metadata that exists in an invalid state, but without the ability to actually get invalid data out of it.

@brettcannon
Copy link
Member

More from Donald:

My branch adds a “raw” metadata class, which doesn’t validate at all, and is what you use for round tripping 100% or when you want to get maximum tolerance for invalid data.

It then takes the “Metadata” class, and delays validating data until it’s accessed (although by default it runs the full validation up front, you can just ask it to delay).

So that means that if you ask the metadata class to delay validation, and all you care about is name + version, it will give you that data validated, but if say… classifiers was invalid, it wouldn’t error out

So you get:

  • RawMetadata if you want very minimal validation/processing
  • Metadata with delayed validation if you want to be lenient of invalid metadata in areas you’re not accessing
  • Metadata with upfront validation (the default) if you want to deal only in valid metadata

In my branch, it’s not done, but the intention is that when you emit from Metadata it only will emit valid metadata, if you want to somehow emit invalid metadata, you’d have to do that manually with RawMetadata

@brettcannon
Copy link
Member

I'm not sure if the lazy vs. eager is worth the added work. At that point you could parse the individual parts via the appropriate functions in packaging from RawMetadata. That would make RawMetadata for consuming and Metadata for producing.

@brettcannon
Copy link
Member

I have created #596 to convert author_emails and maintainer_emails to author_email and maintainer_email, respectively, and to Optional[str] because email specs are hard.

@pradyunsg
Copy link
Member Author

FWIW, #574 is likely the PR containing Donald's work.

@brettcannon
Copy link
Member

I'm not sure if the lazy vs. eager is worth the added work.

FYI I thought about this some more and I think we could do something like raw_version and canonical_version if the lazy approach was desired. But I really don't want to have both a RawMetadata class that is lazy and a Metadata class that is eager as that feels like duplication of work. I think the options would be:

  • Make Metadata lazy, essentially replicating what we did with name for any other field we have a class for (and some way to forcibly validate upfront, probably by default)
  • Introduce RawMetadata which is basically a dataclass but doesn't validate
  • Introduce a typed dict for the raw case that aligns with what the JSON representation of metadata would be

For the first one we would either need to change Metadata now for that case or remove it before releasing. The latter two options we can do whatever.

Not sure if you have an opinion/feeling on this, @pradyunsg ? I see pros and cons with any of the approaches. I think it comes down to whether we want one class or two, and then after that if we want to make the distinction clear by having the raw data in a sixth format (else the APIs would be very similar, which is good/bad).

@pradyunsg
Copy link
Member Author

I still need to think about this -- I don't feel like I've kept up with the entire discussion on RawMetadata vs Metadata and don't wanna have an uninformed opinion on this.

So... to unblock the release, I say we remove Metadata for now, cut a release and then bring it back once the release is out without major disruptions. That doesn't lock us into the current API (letting us do basically anything with Metadata, including nothing). :)

@brettcannon
Copy link
Member

SGTM!

@pradyunsg
Copy link
Member Author

I'll try to find time in the coming week for that, but anyone is welcome to beat me to it!

@stratakis
Copy link

Hi. Is there any update on the new release?

@brettcannon
Copy link
Member

I personally don't and the fact @pradyunsg didn't get to it suggest he doesn't either. 😉

@pradyunsg
Copy link
Member Author

No updates. I've gotten back from vacation just now and this is pretty high up my list of OSS things to do.

@nanonyme
Copy link

So is 22.0 then intended to be the update when packaging releases with flit as buildsystem?

@brettcannon
Copy link
Member

So is 22.0 then intended to be the update when packaging releases with flit as buildsystem?

Yes as that's already the case in the main branch.

@rossburton
Copy link

Looking forward to a release for both flit and the lack of pyparsing...

@pradyunsg
Copy link
Member Author

@brettcannon I'll take this and cut a release over the weekend. I imagine this being a major version bump isn't gonna be a big pain but, hopefully, no pitchforks are raised once this is released. :)

@pradyunsg pradyunsg self-assigned this Nov 24, 2022
@kasium
Copy link
Contributor

kasium commented Dec 1, 2022

@pradyunsg thanks for working on it. Can you please give any update since many folks are waiting for it 😊

@pradyunsg
Copy link
Member Author

We have a regression in the requirement parsing rewrite (#618), which either needs reverting or fixing.

@nanonyme
Copy link

nanonyme commented Dec 3, 2022

Reverting means pulling back pyparsing, right? I don't understand why it was removed in the first place because it doesn't lead into bootstrap build cycles.

@brettcannon
Copy link
Member

Reverting means pulling back pyparsing, right? I don't understand why it was removed in the first place because it doesn't lead into bootstrap build cycles.

It meant projects vendoring us had to also vendor pyparsing which was a burden on others. More details can be found in the issue where this was all originally discussed.

@pradyunsg
Copy link
Member Author

pradyunsg commented Dec 6, 2022

For me personally -- better error messaging is also a motivator. Notably, we got fairly bad opaque messages out of the pyparsing-based parser (part of why pip hides the details of the InvalidRequirement errors from the end user). Generating good error messages is something that a hand-written parser can do a much better job with.

None the less, this ended up taking my weekend over Lutra/installer/pip's build-system refactoring/PEP 658 on PyPI, so... #624 is a thing that's ready for review. :)

@nanonyme
Copy link

nanonyme commented Dec 6, 2022

@pradyunsg I would have TBH bought it simply based on Brett's explanation but sounds like indeed indeed using custom parser brings quite a lot of extra value. Keep on with the great going. :)

@pradyunsg
Copy link
Member Author

pradyunsg commented Dec 7, 2022

With #624 merged, I've released 22.0 to PyPI. :)

@pradyunsg pradyunsg pinned this issue Dec 7, 2022
@pradyunsg
Copy link
Member Author

I've updated the Read the Docs configuration (https://readthedocs.org/projects/packaging/) to use /stable/ as the default URL that folks get redirected to from https://packaging.pypa.io. This will take a bit of time to propagate.

@pradyunsg
Copy link
Member Author

Closing this out. Please feel welcome to file issues if you spot any regressions (like #629!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants