Skip to content

Add traceback to manager reported error #190

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

Merged
merged 1 commit into from
May 6, 2025

Conversation

H-Huang
Copy link
Member

@H-Huang H-Huang commented May 6, 2025

Before:
image

After:
image

@H-Huang H-Huang requested a review from d4l3k May 6, 2025 15:22
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 6, 2025
@H-Huang H-Huang requested a review from fegin May 6, 2025 15:23
@H-Huang H-Huang force-pushed the diloco_titan branch 3 times, most recently from 2973a5c to 40dc6a4 Compare May 6, 2025 16:31
@@ -444,7 +444,7 @@ def test_pg_errored(self, client_mock: MagicMock) -> None:
manager._pg.errored.return_value = injected_failure

self.assertFalse(manager.should_commit())
self.assertEqual(manager._errored, injected_failure)
self.assertIsInstance(manager._errored, Exception)
Copy link
Member

Choose a reason for hiding this comment

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

can we check .original_exception here instead? and for all the tests below including raise?

The type of error I think is pretty important for some of these tests to distinguish different code branches

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

@H-Huang H-Huang requested a review from d4l3k May 6, 2025 19:45
Copy link
Member

@d4l3k d4l3k left a comment

Choose a reason for hiding this comment

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

LGTM

@H-Huang H-Huang merged commit 24b2ae8 into pytorch:main May 6, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants