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.

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

Debug mode flag #9428

Merged
merged 2 commits into from
Sep 21, 2021
Merged

Debug mode flag #9428

merged 2 commits into from
Sep 21, 2021

Conversation

MrMino
Copy link
Contributor

@MrMino MrMino commented Jan 4, 2021 •
edited
Loading

Fixes #9349.

Adds --debug flag to CLI.

This is how it works with ipdb -m (it's a bit bloated because of DEBUG but yes, you can now travel up the exception chain post-mortem):

demo

@MrMino MrMino force-pushed the debug_mode_flag branch 2 times, most recently from 37c6168 to 235643a Compare January 4, 2021 23:56
uranusjr
uranusjr reviewed Jan 5, 2021
View reviewed changes
src/pip/_internal/cli/base_command.py Outdated Show resolved Hide resolved
src/pip/_internal/cli/base_command.py Outdated Show resolved Hide resolved
@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 4, 2021
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 4, 2021
@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 20, 2021
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 20, 2021
uranusjr
uranusjr approved these changes Feb 22, 2021
View reviewed changes
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

There should be a follow-up to add this to the documentation.

@MrMino
Copy link
Contributor Author

MrMino commented Feb 22, 2021 •
edited
Loading

@uranusjr you mean it should be a separate PR or can I add it to this one?

@uranusjr
Copy link
Member

Up to you, I’m fine with either.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 22, 2021
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 27, 2021
@MrMino
Copy link
Contributor Author

MrMino commented Feb 27, 2021

@uranusjr looks like it's already handled automatically by the .. pip-general-options:: directive in docs/html/reference/pip.rst.

It looks like this:

image

Would you like me to add something more?

@uranusjr
Copy link
Member

I was thinking about ading a brief write-up in the Development chapter since this would make the option most discoverable by contributors.

I think implementation-wise this PR is good to go, but want another maintainer to merge this to make sure I’m not the only one thinking this is a good idea.

@uranusjr uranusjr added type: enhancement Improvements to functionality type: maintenance Related to Development and Maintenance Processes labels Feb 27, 2021
@MrMino
Copy link
Contributor Author

MrMino commented Feb 27, 2021

Oh, I see. I'll follow up with another PR with a short write-up on this in the docs after this one is merged.

@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Aug 15, 2021
@pradyunsg
Copy link
Member

@MrMino Any chance you'd be willing to update this PR, to resolve the merge conflicts on this?

@pradyunsg pradyunsg added the S: awaiting response Waiting for a response/more information label Sep 18, 2021
@MrMino
Copy link
Contributor Author

MrMino commented Sep 18, 2021

@pradyunsg Sure thing, just give me a day or two.

@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Sep 19, 2021
@MrMino
Copy link
Contributor Author

MrMino commented Sep 19, 2021

@pradyunsg Done. The only change to my hunks is the Any import being put back in L11 of base_command.py:

https://github.com/pypa/pip/pull/9428/files#diff-b94febdf36940df571be1adf55a33cf99ffb14512eef1f05007780d74e3cdfa3R11

(see vimdiff <(git show --patch 3ab977c) <(git show --patch edc6d31))

uranusjr
uranusjr approved these changes Sep 20, 2021
View reviewed changes
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

LGTM generally, please fix the remaining type annotation comment mentioned above.

@MrMino
Add --debug flag to base command
03e0053 
This flag makes the main subroutine (cli.base_command.Command.run)
withold from intercepting unhandled exceptions. This means, that
debugging via "python -m pdb -m pip" is now possible.
@MrMino
Copy link
Contributor Author

MrMino commented Sep 20, 2021

Done.

@uranusjr uranusjr added this to the 21.3 milestone Sep 20, 2021
pradyunsg
pradyunsg approved these changes Sep 21, 2021
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

One minor thing, that I'll do myself prior to merging.

news/9349.feature.rst Outdated Show resolved Hide resolved
@pradyunsg pradyunsg merged commit a8cf21e into pypa:main Sep 21, 2021
@pradyunsg
Copy link
Member

Thanks for doing this @MrMino! ^>^

@MrMino MrMino deleted the debug_mode_flag branch September 21, 2021 22:44
@MrMino
Copy link
Contributor Author

MrMino commented Sep 22, 2021 •
edited
Loading

@uranusjr I guess I should now follow this up with the docs write-up, right? Separate section at the end of Getting started (above "Building Documentation") maybe?

@uranusjr
Copy link
Member

That sounds about right to me.

@MrMino MrMino mentioned this pull request Sep 28, 2021
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jdufresne jdufresne jdufresne left review comments

@pradyunsg pradyunsg pradyunsg approved these changes

@uranusjr uranusjr uranusjr approved these changes

Assignees
No one assigned
Labels
S: awaiting response Waiting for a response/more information type: enhancement Improvements to functionality type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Milestone
21.3
Development

Successfully merging this pull request may close these issues.

Allow troubleshooting with pdb
6 participants
@MrMino @BrownTruck @uranusjr @pradyunsg @jdufresne @pypa-bot