8000 [BE]: Enable RUFF TRY400 rule - log.exception by Skylion007 · Pull Request #153473 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

[BE]: Enable RUFF TRY400 rule - log.exception #153473

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
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[BE]: Enable RUFF TRY400 rule - log.exception #153473

wants to merge 2 commits into from

Conversation

Skylion007
Copy link
Collaborator
@Skylion007 Skylion007 commented May 13, 2025 •
edited by pytorch-bot bot
Loading

Change logging.error to logging.exception to log additional information when relevant. A few places have slipped in logging.errors in try except since I last did a clean up here and the rule is stabilized so I am enabling it codebase wide. I have NOQA'd much of our custom exception stack trace handling for RPC calls and distributed and tried to a fix a few errors based on whether we immediately reraised it or if we didn't print any exception handling where it could be useful.

cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @ezyang @SherlockNoMad @EikanWang @jgong5 @wenzhe-nrv @voznesenskym @penguinwu @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov

@Skylion007 Skylion007 requested review from albanD and atalman May 13, 2025 16:29
@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented May 13, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/153473

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 1 Unrelated Failure

As of commit 398f956 with merge base ba70876 (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@Skylion007 Skylion007 requested a review from malfet May 13, 2025 16:29
@Skylion007 Skylion007 force-pushed the skylion007/enable-TRY400-2025-05-13 branch from 8a50eac to eacb44d Compare May 13, 2025 16:33
albanD
albanD approved these changes May 13, 2025
View reviewed changes
Copy link
Collaborator
@albanD albanD left a comment

Choose a reason for hiding this comment

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

Nice cleanup, thanks!

@Skylion007 Skylion007 force-pushed the skylion007/enable-TRY400-2025-05-13 branch from eacb44d to 1038736 Compare May 14, 2025 14:14
@Skylion007 Skylion007 requested a review from a team as a code owner May 14, 2025 14:14
@Skylion007
Copy link
Collaborator Author

@pytorchbot merge -i

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 14, 2025
@Skylion007 Skylion007 added the better-engineering Relatively self-contained tasks for better engineering contributors label May 14, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: Validate Runner Determinator Script is in Sync / check-runner-determinator

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@pytorchmergebot
Copy link
8000 Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@Skylion007
Copy link
Collaborator Author

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: Validate Runner Determinator Script is in Sync / check-runner-determinator

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@Skylion007 Skylion007 force-pushed the skylion007/enable-TRY400-2025-05-13 branch from da8b39a to 398f956 Compare May 14, 2025 16:59
@Skylion007
Copy link
Collaborator Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command
For more information see pytorch-bot wiki.

@S 8000 kylion007
Copy link
Collaborator Author

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: Validate Runner Determinator Script is in Sync / check-runner-determinator

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@Skylion007
Copy link
Collaborator Author

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 2 checks: Validate Runner Determinator Script is in Sync / check-runner-determinator, inductor / unit-test / cuda12.6-py3.10-gcc9-sm86 / test (inductor_cpp_wrapper, 1, 2, linux.g5.4xlarge.nvidia.gpu)

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@jeanschmidt
Copy link
Contributor

@pytorchbot revert -m "seems to have broken internal signals, @albanD may I count on you to help the author merge his PR? D74837988" -c ghfirst

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot added a commit that referenced this pull request May 16, 2025
@pytorchmergebot
Revert "[BE]: Enable RUFF TRY400 rule - log.exception (#153473)"
3443627 
This reverts commit 4f4ecc5.

Reverted #153473 on behalf of https://github.com/jeanschmidt due to seems to have broken internal signals, @albanD may I count on you to help the author merge his PR? D74837988 ([comment](#153473 (comment)))
@pytorchmergebot
Copy link
Collaborator

@Skylion007 your PR has been successfully reverted.

@pytorchmergebot pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albanD albanD albanD approved these changes

@cyyever cyyever cyyever approved these changes

@atalman atalman Awaiting requested review from atalman

@bobrenjc93 bobrenjc93 Awaiting requested review from bobrenjc93 bobrenjc93 is a code owner

@laithsakka laithsakka Awaiting requested review from laithsakka laithsakka is a code owner

@avikchaudhuri avikchaudhuri Awaiting requested review from avikchaudhuri avikchaudhuri is a code owner

@tugsbayasgalan tugsbayasgalan Awaiting requested review from tugsbayasgalan tugsbayasgalan is a code owner

@zhxchen17 zhxchen17 Awaiting requested review from zhxchen17 zhxchen17 is a code owner

@ydwu4 ydwu4 Awaiting requested review from ydwu4 ydwu4 is a code owner

@angelayi angelayi Awaiting requested review from angelayi angelayi is a code owner

@malfet malfet Awaiting requested review from malfet

Assignees
No one assigned
Labels
better-engineering Relatively self-contained tasks for better engineering contributors ci-no-td Do not run TD on this PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request fx Merged module: dynamo module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue open source release notes: distributed (checkpoint) release notes: distributed (ddp) release notes category Reverted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Skylion007 @pytorchmergebot @jeanschmidt @albanD @cyyever @facebook-github-bot @pytorchbot
0