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.

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

bpo-38780: Avoid errors in SyslogHandler when socket is not set up. #23661

Closed
wants to merge 3 commits into from
Closed

bpo-38780: Avoid errors in SyslogHandler when socket is not set up. #23661

wants to merge 3 commits into from

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Dec 5, 2020

…upply a NullSocket that has no behavior for the functions called.
@vsajip
Copy link
Member

vsajip commented Dec 7, 2020

Why is it better to have a NullSocket rather than None? It seems like None would catch more error cases than a NullSocket which accepts an erroneously made method call which then just does nothing.

@jaraco
Copy link
Member Author

jaraco commented Dec 9, 2020

Why is it better to have a NullSocket rather than None? It seems like None would catch more error cases than a NullSocket which accepts an erroneously made method call which then just does nothing.

I guess my thinking was that because the None required an if None around each invocation of it, it would be better to embed that Noneness into the underlying object. The point of the PR is to suppress errors when the handler is not fully configured. I was tempted for the NullSocket to be even less fitting to the implementation, and using something like a MagicMock or an object that accepts any interface with a trivial response, but ultimately settled on this narrow fit to cover the known needed cases.

@jaraco
Copy link
Member Author

jaraco commented Dec 31, 2020

After some analysis and attempting to write tests, I determined that this approach is inadequate, so I'm closing this PR, although I'll leave the branch and test branch around.

@jaraco jaraco closed this Dec 31, 2020
@jaraco jaraco deleted the bugfix/bpo-38780 branch Mar 5, 2021
@jaraco
Copy link
Member Author

jaraco commented Mar 5, 2021

I've moved the branch to /refs/archive/bugfix/[bpo-38780](https://bugs.python.org/issue38780) in jaraco/cpython.

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

Successfully merging this pull request may close these issues.

None yet

4 participants