Skip to content

Conversation

kshitij12345
Copy link
Collaborator

Fixes #62501

Another approach for fixing the same issue

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 7, 2021

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 1dee3ad (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@kshitij12345 kshitij12345 changed the title [fix] fix test_python_dispath with pytest [fix] fix test_python_dispatch with pytest Sep 7, 2021
@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #64574 (1dee3ad) into master (be09195) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #64574      +/-   ##
==========================================
- Coverage   66.65%   66.65%   -0.01%     
==========================================
  Files         714      714              
  Lines       92546    92546              
==========================================
- Hits        61685    61682       -3     
- Misses      30861    30864       +3     

@kshitij12345 kshitij12345 marked this pull request as ready for review September 7, 2021 17:16
@kshitij12345 kshitij12345 requested a review from ezyang September 7, 2021 17:16
@mrshenli mrshenli added module: dispatch DispatchStub, Type, void pointer table, c10 dispatch triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Sep 7, 2021
@kshitij12345 kshitij12345 requested a review from zou3519 September 8, 2021 05:14
@ezyang
Copy link
Contributor

ezyang commented Sep 8, 2021

@kshitij12345 thanks for opening the conversation here.

Is my understanding correct that the problem is that pytest installs its own log handler, and then our logs here propagate to their log handling which chokes on the full objects? In that case, it seems to me a more straightforward fix would be just to prevent propagation. (We're logging to a specific named logger so disabling propagation for this name only seems fine.)

@kshitij12345 kshitij12345 force-pushed the fix/test_python_dispatch-2 branch from ebe66cf to 1d701d1 Compare September 10, 2021 13:54
@kshitij12345
Copy link
Collaborator Author

Did not know about this. Should be ready now. Thank you very much @ezyang!

@facebook-github-bot
Copy link
Contributor

@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in d46ea03.

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

Labels

cla signed Merged module: dispatch DispatchStub, Type, void pointer table, c10 dispatch open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_python_dispatch fails under pytest

5 participants