[issue41857] Document timeout arguments to poll() in select module
Change by Zane Bitter : -- keywords: +patch pull_requests: +21446 stage: -> patch review pull_request: https://github.com/python/cpython/pull/22406 ___ Python tracker <https://bugs.python.org/issue41857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41857] Document timeout arguments to poll() in select module
New submission from Zane Bitter : The select.poll.poll() and select.devpoll.poll() methods don't have their timeout arguments documented in their docstrings. In both cases the argument is in milliseconds, whereas the timeout arguments to select.kqueue.control() and select.epoll.poll() are in seconds, and are documented in the docstring. -- assignee: docs@python components: Documentation messages: 377473 nosy: docs@python, zaneb priority: normal severity: normal status: open title: Document timeout arguments to poll() in select module type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue41857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37857] Setting logger.level directly has no effect due to caching in 3.7+
Zane Bitter added the comment: In turns out that setting the level directly is used in the standard library, so we definitely have a bug here that needs fixing in 3.7 & 3.8. Obviously the fix could be just to stop doing that in the standard library, but I'd argue that this is even stronger evidence that it can happen to anyone, and that we should fix it for Python users in general. > But it's not a priority to support situations where people don't follow > published APIs and still expect things to work the way they would like. That's fair, of course. This isn't and shouldn't be anyone's number 1 priority, but that doesn't mean we can't do it. > I would prefer people to use setLevel(), as it helps to have consistent usage. Me too. I added a new patch to the PR that deprecates the setter so that anyone doing it wrong will get a warning. (In fact that's how I discovered it was being set directly in the standard library.) > The philosophy is that of not breaking userspace code *which follows > published APIs*. If you use unpublished APIs or rely on implementation > details, you're generally on your own. I was referring to stuff like https://devblogs.microsoft.com/oldnewthing/20031015-00/?p=42163 (I won't link to a famous equivalent in Linux because it involves Linus swearing at people), where the users are clearly wrong but they have a policy of not breaking them anyway. -- ___ Python tracker <https://bugs.python.org/issue37857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37857] Setting logger.level directly has no effect due to caching in 3.7+
Zane Bitter added the comment: > It feels wrong to do this as a band-aid to help out people who didn't do the > right thing. I mean... are we here to punish people for making mistakes by sending them to schroedinbug purgatory, or are we here to help them upgrade to the latest Python without feeling like it's a minefield? I genuinely don't know what Python's stance on this is. Certainly the Linux kernel and Windows both have a philosophy of not breaking working userspace code no matter how wrong it might be, so it's not unheard-of to choose the latter. I suggested on the PR that we could deprecate the setter so that people who are Doing It Wrong will get a warning that they can much more easily track down and fix, and we won't encourage any wrong new code. Would that change your opinion? > > but the good news is that the values are now cached > That's not relevant to the performance numbers I posted above, is it? Not directly - your performance numbers show that accessing the attribute is ~300ns slower when it's a property. But the thing that gets called all the time that we want to be fast is isEnabledFor(), and the results of that are cached so it only calls getEffectiveLevel() (which is the thing accessing the attribute) once. Note that populating the cache is already considerably more expensive than the previous path (it requires handling an exception and acquiring a lock), but it's still worth it because the cost is typically amortised across many log calls. The addition of an extra property lookup in this path is not going to be noticeable. -- ___ Python tracker <https://bugs.python.org/issue37857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37857] Setting logger.level directly has no effect due to caching in 3.7+
Zane Bitter added the comment: > That code in the wild that sets the level attribute directly is wrong and > should be changed, right? It definitely should be changed (and in our case we did change it once we found it). Whether it's "wrong" is more of a philosophical question ;). But the problem with that is that it's very difficult to find - the object's state appears to be set OK but the behaviour doesn't match because of the hidden cache with different values. > I'm not sure such misuse is widespread. There's no way to be sure. But if you have a public (i.e. non-underscore-prfixed) attribute then some percentage of people are going to set it, particularly if it seems to work. I went back and looked at how this was introduced in the example I linked, and the patch was reviewed by the guy who literally wrote the book on the Python standard library. That suggests to me that this could happen to anyone. The code doesn't look wrong, so mistakes are going to happen. > I don't know what the real-world performance impact would be Me neither, but the good news is that the values are now cached :) So it doesn't impact the performance of logging a message at all. It will have some impact on the performance of rebuilding the cache, but I'd expect it to be negligible in the context of the other work required to rebuild the cache (which includes obtaining a lock). -- ___ Python tracker <https://bugs.python.org/issue37857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37857] Setting logger.level directly has no effect due to caching in 3.7+
Change by Zane Bitter : -- keywords: +patch pull_requests: +15008 stage: -> patch review pull_request: https://github.com/python/cpython/pull/15286 ___ Python tracker <https://bugs.python.org/issue37857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue30962] Add caching to logging.Logger.isEnabledFor()
Change by Zane Bitter : -- pull_requests: +15009 pull_request: https://github.com/python/cpython/pull/15286 ___ Python tracker <https://bugs.python.org/issue30962> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37857] Setting logger.level directly has no effect due to caching in 3.7+
New submission from Zane Bitter : This is a related issue to bug 34269, in the sense that it is also due to the caching added by the fix for bug 30962. In this case, the bug is triggered by setting the public 'level' attribute of a logging.Logger object, instead of calling the setLevel() method. Although this was probably never a good idea, prior to Python3.7 it worked as expected. Now it renders the level out of sync with the cache, leading to inconsistent results that are hard to debug. An example in the wild: https://review.opendev.org/676450 -- components: Library (Lib) messages: 349741 nosy: zaneb priority: normal severity: normal status: open title: Setting logger.level directly has no effect due to caching in 3.7+ type: behavior versions: Python 3.7, Python 3.8 ___ Python tracker <https://bugs.python.org/issue37857> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue28603] traceback module can't format/print unhashable exceptions
Zane Bitter <za...@fedoraproject.org> added the comment: I also encountered this issue, and I'd like to further note that it is extraordinarily difficult to debug without access to stderr. If your logging facility uses the traceback module then it will be unable to record both the original (unhashable) exception, and the TypeError that it triggers (since the original exception is its __context__). The effect is that execution of a thread appears to simply stop without rhyme or reason (in fact it is still executing, but that fact is not apparent from the logs). I'm attaching a short reproducer that demonstrates this effect. I believe the trade-offs inherent in the attached patches (either stopping the chain at an unhashable exception, or using a less-efficient data structure) can be avoided by comparing for identity rather than equality. The purpose of the check is to avoid an infinite loop; whether the objects compare as equal is something that is up to the author of the class, and has no relevance here except insofar as that objects ought to compare equal to themselves. I submitted a pull request (#4014) with an implementation of that. -- nosy: +zaneb Added file: https://bugs.python.org/file47223/hashex.py ___ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue28603> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue28603] traceback module can't format/print unhashable exceptions
Change by Zane Bitter <za...@fedoraproject.org>: -- pull_requests: +3988 ___ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue28603> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
