Skip to content

[3.12] gh-96747: Add passlib package to crypt doc depraction warning (GH-106660) #106660

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 12, 2023
Merged

[3.12] gh-96747: Add passlib package to crypt doc depraction warning (GH-106660) #106660

merged 3 commits into from
Jul 12, 2023

Conversation

bityob
Copy link
Contributor

@bityob bityob commented Jul 12, 2023 •
edited
Loading

This PR add details about passlib package on the crypt doc.

I didn't first add this change to main branch since this crypt.rst file and the module are deleted on 3.11 version.

📚 Documentation preview 📚: https://cpython-previews--106660.org.readthedocs.build/

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir skip news labels Jul 12, 2023
@ghost
Copy link

ghost commented Jul 12, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bityob bityob changed the title gh-96757: Add passlib package to crypt doc depraction warning [3.11] gh-96747: Add passlib package to crypt doc depraction warning [3.11] Jul 12, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jul 12, 2023
Closed
@bityob bityob force-pushed the bityob-fix-doc-crypt-gh-96747-for-python-3-11 branch from a2ec26b to d102845 Compare July 12, 2023 02:11
@bityob bityob changed the title gh-96747: Add passlib package to crypt doc depraction warning [3.11] gh-96747: Add passlib package to crypt doc depraction warning Jul 12, 2023
@bityob bityob changed the title gh-96747: Add passlib package to crypt doc depraction warning [3.11] gh-96747: Add passlib package to crypt doc depraction warning (GH-106660) Jul 12, 2023
@bedevere-bot
Copy link

GH-106660 is a backport of this pull request to the 3.11 branch.

@bityob
Copy link
Contributor Author

bityob commented Jul 12, 2023

Hey @iritkatriel,

This PR is needed only for 3.11 and 3.12 versions and can't be added to main branch since this file will be deleted for 3.13 version.

Because of that I had to use this specific PR title, where the linked PR (GH-NNNNN) is the current one and not from the main as it supposed to be.

How should I do the process?

Is it fine to add to 3.11 and later cherry-pick to 3.12?

Can you review?

Thanks

@bityob bityob marked this pull request as ready for review July 12, 2023 02:24
@iritkatriel iritkatriel requested a review from hugovk July 12, 2023 07:53
@iritkatriel
Copy link
Member

Is it fine to add to 3.11 and later cherry-pick to 3.12?

It's fine. I would probably start with the newest branch (3.12) and try an automatic backport, but doing it manually on both branches is also fine.

Can you review?

I am not a domain expert, but I have asked other core devs to confirm we are happy to make this recommendation. We'll get back to you.

@iritkatriel
Copy link
Member

Note that there is a failure in the doctest buildbot:

NameError: name 'des_crypt' is not defined

hugovk
hugovk reviewed Jul 12, 2023
View reviewed changes
@hugovk
Copy link
Member

hugovk commented Jul 12, 2023

passlib hasn't been updated in a couple of years, but it might still be fine?

Let's also check with @gpshead who gave the original issue a 👍

@iritkatriel iritkatriel requested a review from ambv July 12, 2023 13:40
@bityob
Copy link
Contributor Author

bityob commented Jul 12, 2023

Note that there is a failure in the doctest buildbot:

NameError: name 'des_crypt' is not defined

Fixed

gpshead
gpshead requested changes Jul 12, 2023
View reviewed changes
Copy link
Member

@gpshead gpshead left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This PR needs to be made against the main branch. (or... 3.12 if the doc was already removed in main)

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@gpshead
Copy link
Member

gpshead commented Jul 12, 2023

as to the main branch... if the docs are removed there already, make the PR against 3.12 instead of 3.11. :)

@gpshead
Copy link
Member

gpshead commented Jul 12, 2023

passlib hasn't been updated in a couple of years, but it might still be fine?

Let's also check with @gpshead who gave the original issue a 👍

While a little annoying to see it lingering for a few years with no activity from the single author/maintainer, it does seem like the right tool for this job compared to anything else out there. If it ever turns out not to be, we can simply update our docs again in the future.

@bityob bityob force-pushed the bityob-fix-doc-crypt-gh-96747-for-python-3-11 branch from c6afc3a to 3a622f0 Compare July 12, 2023 19:47
@bityob bityob requested review from brettcannon, a team and encukou as code owners July 12, 2023 19:47
@hugovk hugovk added the needs backport to 3.11 only security fixes label Jul 12, 2023
@bityob
Copy link
Contributor Author

bityob commented Jul 12, 2023 •
edited
Loading

A PR must be based on the version it was branched from. If, as appears, this PR was based on a branch of 3.11, you cannot just rebase on 3.12, as it ruins the diff and generates all the review requests. You must create a new PR branch by switching to 3.12 first and then switch to a new branch.

Hey @terryjreedy,

Sorry for those all review requests, didn't know it will do so...

Regarding the code, I rebased my commits on top of 3.12 and the diff is fine now.

Thanks

@bedevere-bot
Copy link

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from gpshead July 12, 2023 20:34
@gpshead gpshead merged commit ec7b05a into python:3.12 Jul 12, 2023
@miss-islington
Copy link
Contributor

Thanks @bityob for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 12, 2023
@bityob @miss-islington
[3.12] pythongh-96747: Mention the PyPI passlib package in the `cry…
b5aaa6e 
…pt` deprecation doc (pythonGH-106660) (pythonGH-106660)

* Added mention to passlib package as alternative to the deprecated crypt module.
(cherry picked from commit ec7b05a)

Co-authored-by: Yonatan Bitton <bityob@gmail.com>
@bedevere-bot
Copy link

GH-106697 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 12, 2023
@gpshead
Copy link
Member

gpshead commented Jul 12, 2023

Regarding the code, I rebased my commits on top of 3.12 and the diff is fine now.

I think this is the first time I've seen a PR successfully be rebased against another branch. 😅 I'm used to just creating a new PR. thanks for the doc update!

gpshead pushed a commit that referenced this pull request Jul 12, 2023
@miss-islington @bityob
[3.11] [3.12] gh-96747: Mention the PyPI passlib package in the `cr…
eac0616 
…ypt` deprecation doc (GH-106660) (GH-106660) (#106697)

[3.12] gh-96747: Mention the PyPI `passlib` package in the `crypt` deprecation doc (GH-106660) (GH-106660)

* Added mention to passlib package as alternative to the deprecated crypt module.
(cherry picked from commit ec7b05a)

Co-authored-by: Yonatan Bitton <bityob@gmail.com>
@gpshead gpshead self-assigned this Jul 12, 2023
@bityob
Copy link
Contributor Author

bityob commented Jul 12, 2023

Regarding the code, I rebased my commits on top of 3.12 and the diff is fine now.

I think this is the first time I've seen a PR successfully be rebased against another branch. 😅 I'm used to just creating a new PR. thanks for the doc update!

😄

It toke me indeed few minutes to figure out how to do it correctly.

Usually git rebase -Xours 3.12 would be enough to take only the file I changed and not something else from 3.11, but this time it wasn't enough.

Finally I found out that the right command for my case is git rebase --onto 3.12 3.11 bityob-fix-doc-crypt-gh-96747-for-python-3-11 which worked like a charm! 🪄

git rebase --onto

image

Source: https://git-scm.com/docs/git-rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugovk hugovk hugovk left review comments

@gpshead gpshead gpshead approved these changes

Assignees

@gpshead gpshead

Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@bityob @bedevere-bot @iritkatriel @hugovk @gpshead @terryjreedy @miss-islington