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

PEP 715: Disabling bdist_egg distribution uploads on PyPI #3161

Merged
merged 25 commits into from
Jun 9, 2023

Conversation

woodruffw
Copy link
Contributor

@woodruffw woodruffw commented Jun 7, 2023

This is a new draft proposal for a packaging PEP.

As a high-level summary: this PEP proposes deprecating and then removing upload support for Egg distributions from PyPI, as identified by the bdist_egg filetype and/or the .egg distribution filename extension.

The PEP proposes a 6 month deprecation period, during which time Egg distributions will continue to be allowed. During the deprecation period, packages that upload an Egg distribution will receive an email warning them about the upcoming deprecation. At the end of the deprecation period, Egg uploads will be permanently disabled.

Existing Egg uploads will continue to be served by PyPI, and this PEP does not affect them.

Additional rationale for this PEP is present in pypi/warehouse#10653.


Basic requirements (all PEP Types)

  • Read and followed PEP 1 & PEP 12
  • File created from the latest PEP template
  • PEP has next available number, & set in filename (pep-NNNN.rst), PR title (PEP 123: <Title of PEP>) and PEP header
  • Title clearly, accurately and concisely describes the content in 79 characters or less
  • Core dev/PEP editor listed as Author or Sponsor, and formally confirmed their approval
  • Author, Status (Draft), Type and Created headers filled out correctly
  • PEP-Delegate, Topic, Requires and Replaces headers completed if appropriate
  • Required sections included
    • Abstract (first section)
    • Copyright (last section; exact wording from template required)
  • Code is well-formatted (PEP 7/PEP 8) and is in code blocks, with the right lexer names if non-Python
  • PEP builds with no warnings, pre-commit checks pass and content displays as intended in the rendered HTML
  • Authors/sponsor added to .github/CODEOWNERS for the PEP

cc @dstufft @ewdurbin @di


📚 Documentation preview 📚: https://pep-previews--3161.org.readthedocs.build/

Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw woodruffw requested a review from a team as a code owner June 7, 2023 03:07
pep-9999.rst Outdated Show resolved Hide resolved
@CAM-Gerlach CAM-Gerlach added the new-pep A new draft PEP submitted for initial review label Jun 7, 2023
@CAM-Gerlach CAM-Gerlach changed the title Draft PEP: Disabling bdist_egg uploads PEP 715: Disabling bdist_egg distribution uploads on PyPI Jun 7, 2023
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
@CAM-Gerlach CAM-Gerlach self-requested a review June 7, 2023 03:15
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Apparently GH issues are not supported.

Signed-off-by: William Woodruff <william@yossarian.net>
pep-0715.rst Show resolved Hide resolved
Signed-off-by: William Woodruff <william@yossarian.net>
pep-0715.rst Outdated Show resolved Hide resolved
pep-0715.rst Outdated Show resolved Hide resolved
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jun 7, 2023

Hey @woodruffw , could you please avoid renaming the file back and forth in the future? It obsoletes all reviewer comments and suggestions, and will furthermore make comments on in-flight reviews completely disappear into the void, which I've had happen to my half-dozen review suggestions so far twice now (which is, as you might imagine, rather frustrating 🙂).

This is why we've switched to specifically asking in the new PEP checklist that instead of using a placeholder number, you just give the PEP the next available number that it would be assigned anyway (not used by a published or in-PR PEP) yourself from the get go, to avoid the need for a file name, etc. change in all but the unlikely event that your PEP number happens to conflict with another submitted around the same time, or your PEP is not approved for draft publication in its current form—in which case it is no worse than the previous situation of always having to rename at least once. Thanks!

@dstufft
Copy link
Member

dstufft commented Jun 7, 2023

This is why we've switched to specifically asking in the new PEP checklist (in your OP) that instead of using a placeholder number, you just give the PEP the next available number that it would be assigned anyway (not used by a published or in-PR PEP) yourself from the get go, to avoid the need for a file name, etc.

We should probably update PEP 1 if this is the new workflow, It still calls out:

You, the PEP author, fork the PEP repository, and create a file named pep-9999.rst that contains your new PEP. Use “9999” as your draft PEP number.

@woodruffw
Copy link
Contributor Author

woodruffw commented Jun 7, 2023

Hey @woodruffw could you please avoid renaming the file back and forth? It obsoletes all reviewer comments and suggestions, and will furthermore make comments on in-flight reviews completely disappear into the void, which I've had happen to my half-dozen review suggestions so far twice now (which is, as you might imagine, rather frustrating 🙂).

I'm extremely sorry -- I hadn't realized (although should have reasonably expected) that these renames would invalidate in-flight reviews.

I'll avoid any more renames for the time being; again, please accept my sincere apologies for the frustrations!

Edit:

We should probably update PEP 1 if this is the new workflow, It still calls out:

Yeah, this is what I followed, and I didn't see the checklist until @dstufft added it to my OP (thanks again!). I'll use the next-available PEP number in any subsequent PEPs I do.

@ewdurbin
Copy link
Member

ewdurbin commented Jun 7, 2023

Yeah, seems there is a conflict between the checklist, PEP instructions, and the PEP numbering guidance from PEP-1:

Assign a PEP number (almost always just the next available number, but sometimes it’s a special/joke number, like 666 or 3141). (Clarification: For Python 3, numbers in the 3000s were used for Py3k-specific proposals. But now that all new features go into Python 3 only, the process is back to using numbers in the 100s again. Remember that numbers below 100 are meta-PEPs.)

Doesn't seem that a sequential PEP number is required.

@dstufft
Copy link
Member

dstufft commented Jun 7, 2023

We should probably update PEP 1 if this is the new workflow, It still calls out:

#3163

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jun 7, 2023

We should probably update PEP 1 if this is the new workflow, It still calls out:

Yup, we should—as well as a similar line in PEP 12. I'll try to get to it tomorrow, if someone else doesn't first. Seems like you did, while I was writing this comment. Reviewed your PR, thanks!

I'm extremely sorry -- I hadn't realized (although should have reasonably expected) that these renames would invalidate in-flight reviews.

No worries, mistakes happen and you didn't realize—ideally GitHub should track these renames and move comments accordingly, but it doesn't. Thanks for understanding!

Yeah, this is what I followed, and I didn't see the checklist until @dstufft added it to my OP (thanks again!). I'll use the next-available PEP number in any subsequent PEPs I do.

Yup, our oversight there, sorry! Our New PEP checklist is part of the New PEP PR template, but currently (as its an alpha GitHub feature we're testing) it does not actually work if you create the PR from the link GitHub sends you back when you git push, or via some other methods.

Doesn't seem that a sequential PEP number is required.

As the section states, the next sequential number is "almost always" used, with the rare historical exceptions being, as mentioned, "special" or "joke" PEPs with a clear and compelling reason to use the specific number. As far as I'm aware, while there's been a few requests for vanity numbers more recently, including by core devs, there hasn't been a compelling-enough case to deviate from the standard, established pattern in recent times, given it makes the ordering more useful, meaningful, predictable and consistent, and avoids favoritism and potential confusion as to why one PEP gets a "special" out of sequence number while another does not.

@dstufft
Copy link
Member

dstufft commented Jun 7, 2023

I've never had the template actually work TBH, I'm not sure where it's supposed to show up, but I might have just always clicked the link that GitHub puts in my terminal after I push, I don't really remember. I just always copy/paste it manually out of the .github/ directory.

Does the alpha feature work if you have the "old" style pull request template? Would it make sense including a combined template until it does work?

pep-9999.rst Outdated Show resolved Hide resolved
.github/CODEOWNERS Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
@CAM-Gerlach
Copy link
Member

I've never had the template actually work TBH, I'm not sure where it's supposed to show up

It shows up if you click the New pull request button on the PRs list, or you click the button to create a PR that shows up in a callout box on the repo pages right after you've pushed to a branch ("branch-name had recent pushes").

image

I just always copy/paste it manually out of the .github/ directory.

Yeah, I'm used to doing that for reviews, so its not really a huge deal (unless the PRs are merged too quickly, i.e. <24 h or when I'm taking a break like I was for the past month), which is more of a potential problem with accepting/rejecting or marking Final PEPs as opposed to with New PEPs for which there's usually more than enough time to review and the requirements are better-understood and remembered.

Does the alpha feature work if you have the "old" style pull request template? Would it make sense including a combined template until it does work?

Good question. I have no idea, honestly, because there's zero documentation anywhere, and its only enabled for this one repo so there's no way to test it besides pushing it live for everyone (it already took multiple rounds of trial and error debugging to get the current templates where they are now). We considered that option back before we had PR templates, but we didn't want it to be too cumbersome for authors among other reasons.

woodruffw and others added 4 commits June 7, 2023 09:34
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Contributor Author

woodruffw commented Jun 7, 2023

I've added "Backwards Compatibility," "Security Implications," and "How To Teach This" sections per @CAM-Gerlach's recommendations: 54b4eb7

Edit: Sorry, mis-clicked the "close PR" button.

@woodruffw woodruffw closed this Jun 7, 2023
@woodruffw woodruffw reopened this Jun 7, 2023
Rather than referencing.

Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
pep-9999.rst Outdated Show resolved Hide resolved
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some further followup fixes and suggestions, mostly on the added text. Otherwise, LGTM minus the rename, thanks!

pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
pep-9999.rst Outdated Show resolved Hide resolved
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@woodruffw
Copy link
Contributor Author

Thanks for the detailed review @CAM-Gerlach! I left two points open for resolution against what @ewdurbin was thinking here 🙂

Signed-off-by: William Woodruff <william@yossarian.net>
@CAM-Gerlach
Copy link
Member

Hey, just FYI, while the GitHub UI makes it way too overly tempting, there's no need to actually need to keep merging in the latest main unless there is actually a reason to (e.g. a merge conflict or a specific change you need), as it just adds commit noise, extra CI runs (which need to be manually approved on your first PR) and notification pings for everyone. Thanks!

@woodruffw
Copy link
Contributor Author

Good to know, thank you! I'll avoid pressing the (tempting) merge button again 🙂

woodruffw and others added 2 commits June 8, 2023 16:05
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Signed-off-by: William Woodruff <william@yossarian.net>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, minus the rename of the file and in the Codeowners/headers (which you can probably go ahead with now, since there are no other outstanding suggestions). Thanks!

@dstufft , did you have further comments or feedback?

@dstufft
Copy link
Member

dstufft commented Jun 9, 2023

I think it looks good to go for being posted for discussion.

Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Contributor Author

Renamed and retitled!

I assume I should create the discussion topic after merge? That's what PEP 1 indicates 🙂

@dstufft
Copy link
Member

dstufft commented Jun 9, 2023

Either right after or right before is OK. I normally do it right after so the file is live on the peps website.

@JelleZijlstra JelleZijlstra merged commit 78f1d5f into python:main Jun 9, 2023
5 checks passed
@woodruffw woodruffw deleted the ww/no-more-eggs branch June 9, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-pep A new draft PEP submitted for initial review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants