databrickslabs / databrickslabs/impulse

Deprecate query_engine.batch_size in favour of max_selectors_per_batch

Open
#102 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
25
Forks
12
Avg merge
1d 5h
Merged PRs (30d)
13

Description

Problem: The config field query_engine.batch_size (config_parser.py:401, default 500) is opaque. It actually caps the number of unique TimeSeriesSelector instances per solve batch (see build_batches, report_utils.py:51), not a generic row/record count.

This gets worse once PR #98 lands, which adds query_engine.max_containers_per_batch. Two batching dimensions sitting side by side, one named batch_size and one named max_containers_per_batch, is confusing. They should read symmetrically: max_selectors_per_batch and max_containers_per_batch.

Proposal: Rename batch_size to max_selectors_per_batch, keeping batch_size as a deprecated alias for one release.

  • config_parser.py: add max_selectors_per_batch (default 500); accept batch_size as a deprecated alias that resolves to it and emits a DeprecationWarning; error if both are set.
  • Update internal call sites and param names: report.py:913, report.py:936; the batch_size parameter in report_utils.build_batches / the batch helpers; docstring in calculated_channel.py:51.
  • Update tests (report_utils_test.py, config_parser_test.py) and docs.

Done when: configs use max_selectors_per_batch; batch_size still works but warns; deprecation window and removal release are noted in the changelog/docs.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with config_parser.py around line 401 and trace build_batches in report_utils.py, then inspect the call sites in report.py and the documentation in calculated_channel.py. Run report_utils_test.py and config_parser_test.py before changing behavior. Done means the new name is used internally, the old alias warns and conflicts are rejected, and the deprecation window is documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
data-engineering
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.