[Parity]: Add completion reason inference#71
Conversation
ghost
mentioned this pull request
ghost
force-pushed
the
batch-completion-assumption
branch
from
37c1383 to
139c240
Compare
ghost
mentioned this pull request
| items: Iterable[BatchItemStatus], completion_config: CompletionConfig | None = None | ||
| ) -> CompletionReason: | ||
| """ | ||
| Infer completion reason based on batch item statuses and completion config. |
There was a problem hiding this comment.
instead of an arbitrary _helper func, maybe let it live in the BatchResult.from_items() instead, it's not getting used anywhere else.
There was a problem hiding this comment.
I planned on using it with parallel and map for completion to ensure the logic remains identical and isolated 🤔
There was a problem hiding this comment.
there actually already is a thread-safe Counters class in Concurrency, and some logic to calculate the result with that.
I have a suspicion that it might not be so easy to reuse that for this though - the logic is not necessarily exactly the same, but I haven't fully thought it through. tldr; might be that having this "special" case here with it's own reconstruction logic is leaner.
There was a problem hiding this comment.
There's a bit of a difference between this and that, they are concerned with different stages, so it's best to keep this logic here, and when needed to construct BatchResult e.g. due to replay, to use the from_items function directly.
| items: Iterable[BatchItem[R]], | ||
| completion_config: CompletionConfig | None = None, | ||
| ): | ||
| items = list(items) |
- Adds completion reason inference based on the rules outlined in the typescript implementation / spec. fixes: #36
ghost
force-pushed
the
batch-completion-assumption
branch
2 times, most recently
from
afe384b to
fc0992a
Compare
ghost
marked this pull request as ready for review
ghost
force-pushed
the
batch-completion-assumption
branch
from
e7fd080 to
fc0992a
Compare
ghost
mentioned this pull request
|
Closing as duplicate. |
ghost
closed this
2 participants
typescript implementation / spec.
fixes: #36