[issue46404] 3.11a4: a small attrs regression
New submission from Tin Tvrtković : We've received a report over at the attrs issue tracker about our test suite failing on Python 3.11. Here's the link: https://github.com/python-attrs/attrs/issues/907 It turns out to be an issue with the no-arg `super()` calls in slotted classes. Here's a minimal reproducer example: ``` from attrs import define @define class A: pass @define class B(A): def test(self): super() B().test() ``` ``` Traceback (most recent call last): File "/Users/tintvrtkovic/pg/attrs/a01.py", line 15, in B().test() ^^ File "/Users/tintvrtkovic/pg/attrs/a01.py", line 12, in test super() ^^^ TypeError: super(type, obj): obj must be an instance or subtype of type ``` This is a known issue for which we have implemented workarounds. The workarounds aren't effective for 3.11 though. I have implemented a fix in attrs (https://github.com/python-attrs/attrs/pull/910), but I still thought I'd post this here to maybe get the core devs opinion. Dataclasses exhibit the exact same issue when used with `slots=True`, both in 3.10 when `slots` was added and in 3.11. I guess no one reported it or tried fixing it. A comprehensive description of the issue follows: since it's impossible to add *slotness* (i.e. set `__slots__`) to a class after it has been created, when creating a slotted class the class decorators in attrs and dataclasses actually replace the class they are applied to with a copy of it, with slots added. This works, except in the case of the no-arg `super()` being used in any of the class methods (and maybe another edge case that I can't remember). When the compiler encounters the no-arg `super()` form, it adds some state to the function `__closure__` cells. This state causes the exception shown above, since it's incorrect when the class gets replaced. So these closure cells need to be rewritten when the class is replaced. In Python versions prior to 3.11, the closure cells were immutable so extra effort was needed to rewrite them. The functions are here: https://github.com/python-attrs/attrs/blob/9727008fd1e40bc55cdc6aee71e0f61553f33127/src/attr/_compat.py#L145. In 3.11, our old closure cell rewriting doesn't work any more, but closure cells don't appear to be immutable either, so the fix in my attr PR linked above is simple. Still, it's another branch in the code to support a specific version. I don't know if there's anything actionable here for Python, apart from confirming or denying if this behavior is expected. -- components: Interpreter Core messages: 410730 nosy: tinchester priority: normal severity: normal status: open title: 3.11a4: a small attrs regression type: behavior versions: Python 3.11 ___ Python tracker <https://bugs.python.org/issue46404> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46200] Discourage logging f-strings due to security considerations
Tin Tvrtković added the comment: Eric is absolutely right, due to function calls being somewhat slow in Python the performance argument in practice falls in favor of f-strings. So if they're faster, more readable, and more convenient to write, no wonder people prefer them (including me). The overload approach is intriguing. -- nosy: +tinchester ___ Python tracker <https://bugs.python.org/issue46200> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46200] Discourage logging f-strings due to security considerations
Tin Tvrtković added the comment: I mean, I agree with your point about being able to accidentally format twice when using the standard library logger. I'm not a core dev but I think getting new APIs in will be challenging. And if by some fluke of chance you did get the approval to introduce new logging APIs, there are better APIs out there (look at the structlog library). Also the old APIs would have to stay for a *long* time since everybody's using them. Adding clever typing overloads seems actually realistic and useful to me. But that depends on the LiteralString PEP being accepted and people actually running MyPy. That's probably the best we can do, apart from some documentation. -- ___ Python tracker <https://bugs.python.org/issue46200> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: I've essentially forked `async-timeout` (a very good library) into Quattro cancel scopes: https://github.com/Tinche/quattro/blob/main/src/quattro/cancelscope.py. The differences are: * the API follows Trio, so instead of `timeout` you'd use `fail_after` or `move_on_after` * instead of `async with timeout`, you use a normal context manager `with fail_after`. The Trio folks think this is important (less suspension points, less race conditions) and I agree * it's somewhat composable (as much as possible under asyncio), each scope knows if the CancelError is meant for it or should be propagated further. This is implemented by using the CancelError message to carry a nonce. This only works on 3.9+, but here that's not a problem * small deadline adjustment differences, I use a setter on the deadline instead of `update` and `shift` * it's fully type annotated, but so is Andrew's Let me know if this sounds interesting. -- nosy: +tinchester ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: Oh, and Trio's `current_effective_deadline` is also in. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: The use of the cancel message is a hack, yeah. But it's what I had to work with. We could introduce a new, proper cancellation context to Tasks instead, which could be attached to the CancelError when it's raised. On the topic of multiple cancellations being applied to a task before it gets to run: could we just treat the cancellation context as a stack, and when the task gets to run, we throw them all in, one by one? Other options would involve somehow figuring out what the highest priority cancellation context is, or combining all the cancellation contexts into the CancelError somehow. On the topic of a task getting to run at least once before being cancelled: sure, I guess. I've personally never needed this but I can see how it'd be useful. Either have a flag on the Task instance or build that logic into the cancellation context handling? -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: On the topic of TaskGroups needing to know whether to swallow a child CancelledError or no: just use the same nonce/cancellation context trick? I remember asking Nathaniel about why in Trio nurseries and cancel scopes were linked (nurseries contain a cancel scope there), whereas in Quattro they are completely separate, and not really understanding the answer. I think I'm getting it now. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: Ok, first the happy path. We have a task running this: async def a(): with move_on_after(5): await a_long_op() await something_else() `move_on_after` has scheduled a callback to run that calls `parent_task.cancel(msg=1)` 5 seconds after it was executed. So now 5 seconds pass, the callback cancels the task, and `move_on_after` catches the CancelledError, sees the msg == 1, and swallows it. `something_else()` now runs. All good. Sad path. Same scenario, except the event loop is kinda busy since we're running in production. Turns out this task was spawned by a web server, and there's a 5 second timeout (or the client disconnected, or something else). So now we have 2 callbacks that want to cancel this task: the one from `move_on_after` and the one from the web server. The one from the web server is more important, since it's a higher level cancellation. But the callback from `move_on_after` runs first, and marks the task for cancellation, and sets the message to 1. Then, before the task gets to run, the webserver also cancels the task. But that does nothing: https://github.com/python/cpython/blob/6f1efd19a70839d480e4b1fcd9fecd3a8725824b/Lib/asyncio/tasks.py#L206. So now the task actually gets to run, `move_on_after` swallows the CancelledError, and something_else() gets to run. But ideally, it shouldn't. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: @Guido Imagine something_else() takes a long time and a lot of server resources (like a heavy query). If the web server disconnected a tad later and avoided the race condition, the task would have gotten cancelled very soon after the start of something_else() and stopped running it. But since the race did happen, the query avoids cancellation (potentially ~forever). -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: @Guido @Chris Instead of using the message for the nonce we could have a dedicated field for it. I have a proposal though. It's about putting a little logic into the cancellation nonce handling. Let's define the nonce as a float. If you just call Task.cancel(), the nonce defaults to 0.0. We add an argument to Task.cancel, so you can give it a nonce: task.cancel(nonce=nonce). The cancel() method has logic to keep track of the nonce with the smallest value. When it's time for the CancelledError to be thrown into the task, this nonce (default 0.0 if not set by anthing) is attached to the error. Then we change `move_on_after` (and all siblings) to do the following: * in `__enter__`, use time.monotonic() to generate it's own nonce and remember it * in `__exit__`, if the CancelledError has a nonce that is less than its own nonce, it propagates it, otherwise it handles it. How this fixes the sad path in my example: Both the web server and `move_on_after` cancel the task. The web server just calls `task.cancel()`, `move_on_after` calls `task.cancel(nonce=self.nonce)`. No matter the cancellation ordering, the nonce will end up set to 0.0. `move_on_after` will see the 0.0 nonce and propagate the error correctly to kill the task completely. This also handles nested cancel scopes. I'm not sure how it works with the task catching the cancel to do a little cleanup. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: Hello Andrew, here's some followup. About the names: the move_on_after and fail_after names are from Trio. I don't have strong feeling about them at all, whatever most people like. About move_on_after vs loop.call_at and loop.call_later: move_on_after is much more usable in practice, since it doesn't require splitting your coroutine in several. And it's very useful in production code; when dealing with 3rd party APIs you want to wait a reasonable time and continue on your way if the 3rd party API stalls. About `async with` vs `with`: I have strong feelings for `with`. It's not a performance issue; I don't care about that. `async with` to me says there are suspension points involved; this coroutine might or might not be suspended either entering or existing. With a bare `with` I *know* there is no suspension. This is important to keep my concurrent code more correct, which is hard enough. About overriding the task factory: the default Task implementation is implemented in C, correct? I would be way too scared of putting my (Python) implementation in there because of performance. Spending years shaving microseconds off in my other libraries to risk losing it all because I want better cancellation in asyncio would feel really bad. Ideally we can get a good-enough solution in the stdlib so users don't have to do this. About your point 3, first graph: you are right if both context managers propagate exceptions. If the inner one is set to swallow (`move_on_after`) OR the user plans on swallowing the inner one, the problem is *not* solved (or at least I don't understand the solution). This is the code sample from https://bugs.python.org/issue46771#msg413368. And I think swallowing is an important use case, as I've already mentioned. About the special field for nonce: I'm OK with smuggling the nonce in the message. But I stand by my proposal for making the nonce a monotonic number, and that would require a special field to be clean. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: Sure, I'll be glad to work with Andrew on getting something presentable. Going through the discussion in the issue: * it seems like folks don't think move_on is useful enough to be in the stdlib, I understand that point. Users can always catch the timeout error from `timeout`, and I can just keep `move_on` in Quattro. We can always add it later. So as far as I'm concerned we can drop it. * `async with` vs `with`: if Andrew thinks `async with` is easier to teach and less error-prone, I'm ok with having the `async with` civilian version in the stdlib and I can keep the `with` expert versions in Quattro, no problem there. So I'm most interested in the cancellation semantics, because those will be very hard to fix in a 3rd party package. @Andrew, in your schema for the third case the behavior is wrong, the `.cancel()` should win over the timeout. Otherwise using the context manager becomes too risky in real-world situations. I also think your first graph has an issue if the user has a `try/except TimeoutError` between `timeout-a` and `timeout-b`, which is now more probable since we're dropping `move_on`. We can take the discussion to the forked repo; I can put together some tests if that would make it easier. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Change by Tin Tvrtković : -- pull_requests: +29561 pull_request: https://github.com/python/cpython/pull/31415 ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Tin Tvrtković added the comment: @Alex you can follow along here: https://github.com/python/cpython/pull/31394 With the cancel_counter approach, a context manager knows whether to handle or propagate the exception by examining its own local state and the remaining counter on the task. If after uncancelling the counter is still non-zero, it propagates. -- ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Change by Tin Tvrtković : -- pull_requests: +29584 pull_request: https://github.com/python/cpython/pull/31434 ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Change by Tin Tvrtković : -- pull_requests: +29613 pull_request: https://github.com/python/cpython/pull/31483 ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Change by Tin Tvrtković : -- pull_requests: +29635 pull_request: https://github.com/python/cpython/pull/31508 ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46771] Add some form of cancel scopes
Change by Tin Tvrtković : -- pull_requests: +29640 pull_request: https://github.com/python/cpython/pull/31513 ___ Python tracker <https://bugs.python.org/issue46771> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46752] Introduce task groups to asyncio and change task cancellation semantics
Change by Tin Tvrtković : -- nosy: +tinchester nosy_count: 6.0 -> 7.0 pull_requests: +29664 pull_request: https://github.com/python/cpython/pull/31513 ___ Python tracker <https://bugs.python.org/issue46752> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46752] Introduce task groups to asyncio and change task cancellation semantics
Change by Tin Tvrtković : -- pull_requests: +29682 pull_request: https://github.com/python/cpython/pull/31559 ___ Python tracker <https://bugs.python.org/issue46752> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46843] PersistentTaskGroup API
Tin Tvrtković added the comment: The asyncio TaskGroup already uses a WeakSet for its children, so it's suitable for long-lived use. As for errors in siblings aborting the TaskGroup, could you apply a wrapper to the scheduled coroutines to swallow and log any errors yourself? Apart from the timeouts, that should get you a long way towards what you're describing. -- ___ Python tracker <https://bugs.python.org/issue46843> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue44188] ThreadPoolExecutor unbalanced semaphore count
Tin Tvrtković added the comment: I was trying to instrument one of our executors to expose some metrics, so I tried using the value of the semaphore as the number of idle threads and I noticed it was way too large. If this was fixed getting these metrics would be easier. -- nosy: +tinchester ___ Python tracker <https://bugs.python.org/issue44188> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue42332] add __weakref__ to types.GenericAlias
New submission from Tin Tvrtković : For example, dict[int, int] cannot be used with singledispatch because types.GenericAlias doesn't support weak references. I'm an author of a third party library (https://github.com/Tinche/cattrs) for which this functionality would be useful. Here's a similar issue in typing (note that this issue is for *types*.GenericAlias) that was fixed: https://github.com/python/typing/issues/345 -- messages: 380816 nosy: gvanrossum, tinchester priority: normal severity: normal status: open title: add __weakref__ to types.GenericAlias type: enhancement versions: Python 3.10, Python 3.9 ___ Python tracker <https://bugs.python.org/issue42332> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue42332] add __weakref__ to types.GenericAlias
Tin Tvrtković added the comment: It would be great if we could get this into 3.9. -- ___ Python tracker <https://bugs.python.org/issue42332> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5207] extend strftime/strptime format for RFC3339 and RFC2822
Changes by Tin Tvrtković : -- nosy: +tinchester ___ Python tracker <http://bugs.python.org/issue5207> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22013] Add at least minimal support for thread groups
Tin Tvrtković added the comment: For your examples, my first instinct would be to use a thread pool executor. It's a nice high level API and can already do the aggregate join. -- nosy: +tinchester ___ Python tracker <http://bugs.python.org/issue22013> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue26664] Misuse of $ in activate.fish of venv
Tin Tvrtković added the comment: Thanks for dealing with this, Brett, your efforts are appreciated. :) On Thu, Jun 30, 2016 at 9:24 PM Brett Cannon wrote: > > Brett Cannon added the comment: > > Activation doesn't "load" the Python interpreter in the venv; all it does > is tweak shell variables to put the interpreter in venv early on PATH. I > personally never use the activate.* scripts and simply directly execute the > Python interpreter, e.g. venv/bin/python. The only other thing the > activation script does that potentially changes semantics is the setting of > $PYTHONHOME which can do manually if you need that specific bit of > functionality. > > You have to realize that there are over 5,000 issues open on > bugs.python.org. We are all volunteers working on Python and we do our > best to get through issues, but we just don't get to everything on the > timeframe people prefer. I realize this issue is important to you, but > basically every reported issue is important to someone so every time we > prioritize anything over something else we are upsetting someone somewhere. > > -- > > ___ > Python tracker > <http://bugs.python.org/issue26664> > ___ > -- ___ Python tracker <http://bugs.python.org/issue26664> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
python-bugs-list@python.org
Tin Tvrtković added the comment: We're actually getting bitten by this in production through the Riak Python client, so this isn't a strictly theoretical problem. -- nosy: +tinchester ___ Python tracker <http://bugs.python.org/issue19542> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17956] add ScheduledExecutor
Tin Tvrtković added the comment: It's a shame this has been stuck in review for 2.5 years, I could really use something like this right now. neologix, why don't you put this up on PyPI for a while, at least? -- nosy: +tinchester ___ Python tracker <http://bugs.python.org/issue17956> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue26664] find a bug in activate.fish of venv of cpython3.6
Tin Tvrtković added the comment: I'm getting this exact issue on Python 3.5 (xenial system installation). All my existing venvs (from before my upgrade to xenial) work. Here's the diff: > diff .venv/bin/activate.fish ../wrapt/.venv/bin/activate.fish 35c35 < set -gx VIRTUAL_ENV "/home/tin/pg/hypothesis/.venv" --- > set -gx VIRTUAL_ENV "/home/tin/pg/wrapt/.venv" 58,59c58,59 < if test -n "$(.venv) " < printf "%s%s%s" "$(.venv) " (set_color normal) (_old_fish_prompt) --- > if test -n "(.venv) " > printf "%s%s%s" "(.venv) " (set_color normal) (_old_fish_prompt) The added dollar signs are the issue. Removing them fixes the problem. For the record: > fish --version fish, version 2.2.0 And here's the actual error message, the same as in the png: > . .venv/bin/activate.fish $(...) is not supported. In fish, please use '(.venv)'. .venv/bin/activate.fish (line 58): if test -n "$(.venv) " ^ from sourcing file .venv/bin/activate.fish called on line 151 of file /usr/share/fish/config.fish in function “.” called on standard input source: Error while reading file “.venv/bin/activate.fish” -- nosy: +tinchester ___ Python tracker <http://bugs.python.org/issue26664> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue26664] find a bug in activate.fish of venv of cpython3.6
Changes by Tin Tvrtković : -- versions: +Python 3.5 ___ Python tracker <http://bugs.python.org/issue26664> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue26664] find a bug in activate.fish of venv of cpython3.6
Tin Tvrtković added the comment: Also I will add I've been using fish for a long time and have never been affected by #26348. -- ___ Python tracker <http://bugs.python.org/issue26664> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com