Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1154 +/- ##
==========================================
+ Coverage 93.42% 93.59% +0.17%
==========================================
Files 54 54
Lines 3771 3858 +87
==========================================
+ Hits 3523 3611 +88
+ Misses 248 247 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5bbadc2 to
033624c
Compare
7322d8e to
7c3d462
Compare
7c3d462 to
6f9300e
Compare
6f9300e to
c2f5a87
Compare
Pijukatel
left a comment
There was a problem hiding this comment.
I am struggling a lot with the variable naming in this PR. Reading this with fresh eyes the variable names gave me a lot of false leads instead of building a helpful mental model.
The usage of word slot here had a very confusing effect on me. One time it was for an asyncio.Condition and the second time for a very custom context manager that uses the first one inside...
On top of it, slot has its own established meaning in Python, but here it is not related in any manner.
… and applies a new limit
Pijukatel
left a comment
There was a problem hiding this comment.
Thanks. It reads way better with new names. Just two more nits, otherwise good.
| class _CachedRunStatus(NamedTuple): | ||
| """Status of a child run as this Actor run last saw it, used to count the run toward the concurrency limit.""" | ||
|
|
||
| status: str |
There was a problem hiding this comment.
This can be a string Literal. The run statuses and LOST on top
| self._max_concurrent_runs: int | None = None | ||
| """How many recorded runs may be active at once, or `None` for no limit.""" | ||
| self._capacity_available = asyncio.Condition() | ||
| """Serializes counting the active runs, and wakes the waiting starts when a run may have finished.""" |
There was a problem hiding this comment.
...when a run may have finished.
Maybe rather
...when a capacity is available.
since the run can also dissapear
|
Btw. not using RecoverableState for persitence? |
Adds
Actor.set_named_child_run_limits(max_concurrent_runs=...). While that many named child runs areREADY,RUNNING,ABORTINGorTIMING-OUT, a namedActor.start,Actor.call,Actor.start_taskorActor.call_taskthat would start or resurrect a run waits for one of them to finish. Reattaching to a recorded run never waits, and runs without arun_namearen't counted.The count comes from the child run registry, so children started before a migration count too. A child's status is fetched again before counting once it's more than 10 seconds old, so a finished run nobody waited on stops counting.
Actor.callandActor.call_taskstop counting their run as soon as it finishes. A start counts toward the limit until its run is recorded, which keeps concurrent starts under different names within the limit.The limit lives in memory, so the Actor sets it again after a migration. Once the parent receives
ABORTING, a start still waiting under the limit raises aRuntimeErrorwithout starting a run. Waiting further would block the abort of the other children.Closes: #1138
✍️ Drafted by Claude Code