I’d like more time to think about this please. -ash
> On 17 Sep 2026, at 10:24, Blain David <[email protected]> wrote: > > Hi all, > > While reviewing the AIP-104 PR > (https://github.com/apache/airflow/pull/62922), Amogh raised a naming > concern about the Dynamic Task Batching API that I would like to settle here > before the PR merges, > since renaming afterwards would be a breaking change. > > ## What `.batch(size=N)` does today > > `task.batch(size=17).iterate(url=urls)` creates 17 mapped task instances > through Dynamic Task > Mapping, and each of them iterates over its share of `urls` in a single task > instance using > Iterable Tasks. The items are distributed round robin: item `i` goes to task > instance `i % 17`. > So `size` is the number of task instances, not a chunk length, and the task > instances do not > receive contiguous slices of the input. > > The round robin scheme is deliberate. The number of task instances has to be > fixed before the > underlying iterable is consumed, because the scheduler needs the mapping > cardinality up front. > With round robin that number is simply `size`. Contiguous chunking, the way > `itertools.batched` > works, would need `ceil(len(items) / size)` task instances, which is > unknowable until a possibly > unbounded or paginated iterable has been fully drained. That would defeat the > purpose of iterating > lazily. > > ## The concern > > The name reads like `itertools.batched(iterable, size)`, where `size` is the > chunk length and the > number of chunks is derived from it. Ours is the opposite: the number of task > instances is given, > and the per-instance share is derived. The class docstring and the Task SDK > docs currently spend a > full paragraph undoing that first impression. A name that carries the meaning > directly would not > need it. > > ## Options > > 1. Keep `.batch(size=N)` and rely on documentation. Lowest churn, but the > mismatch with > `itertools.batched` stays and every new reader has to be corrected. > > 2. Keep the method, rename the parameter: `.batch(count=N)` or > `.batch(tasks=N)`. Keeps the > "Dynamic Task Batching" vocabulary from the AIP, and `count` or `tasks` > says what the number > is. `.batch(tasks=17).iterate(url=urls)` reads as "spread over 17 tasks, > then iterate". > > 3. Rename the method as well, to something that describes partitioning rather > than chunking, for > example `.partition(count=N)`, `.shard(count=N)` or `.spread(across=N)`. > Most descriptive, but > drifts from the AIP's own terminology, and "shard" and "partition" carry > data-engineering > connotations that may suggest contiguous ranges just as much as "batch" > does. > > My preference is option 2 with `count=`. It keeps the API shape and the AIP > terminology, fixes > the misleading part, and costs nothing now because the feature is unreleased. > `size` would be > kept as a rejected alias raising a clear error rather than silently accepted, > so nobody copies > the old spelling from an early draft. > > Unless there are objections or a better name comes up, I will treat this as > lazy consensus in > 72 hours and update the PR, the docs and the AIP page accordingly. > > Thanks, > David > > General (Internal Property) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
