Skip to content

Commit 3ae6825

Browse files
mgazzaclaude
authored andcommitted
feat(annual): let a caller supply the storage backend to annual_cli.main()
StorageBase is already an abstraction — annual_weather.py and annual_tariff.py only ever call self.storage.load()/save(), and AnnualPredictor takes whatever storage it is given — but annual_cli.main() hard-coded StorageLocalFiles. The only way to run the annual tool against a different backend was therefore to fork the CLI, or to reimplement main() and with it the --machine stdout/stderr contract, which is the fiddly part: the stdout redirect that stops a stray print() from corrupting the one-JSON-object stdout a parent process parses. main() now takes storage_factory, called as storage_factory(work_dir, log) — exactly how StorageLocalFiles is constructed, so the default is that class itself and the command line behaves as it always has. The motivating case is embedding the annual tool in a long-lived service. There a per-process work dir means every process re-downloads the same immutable ERA5 and Octopus data and none of it can be shared, which also multiplies requests against the rate APIs. A shared backend (Redis, S3, a database) fixes both, and now needs a factory rather than a fork. Tests cover both paths: the default still constructs StorageLocalFiles, and a supplied factory reaches AnnualPredictor and receives --work-dir and the run's log callable. The new assertions were verified to fail without the change (TypeError: unexpected keyword argument 'storage_factory'). The annual_cli, annual_cli_machine, annual_cli_machine_end_to_end, annual_job and storage suites all pass, and black 23.11.0 and ruff 0.11.4 (the pinned pre-commit versions) report no changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e409e61 commit 3ae6825

2 files changed

Lines changed: 75 additions & 4 deletions

File tree

apps/predbat/annual_cli.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,23 @@ def progress(completed, total, message):
150150
return progress
151151

152152

153-
def main(argv=None):
154-
"""Parse arguments, run the projection, and write the results. Returns an exit code."""
153+
def main(argv=None, storage_factory=StorageLocalFiles):
154+
"""Parse arguments, run the projection, and write the results. Returns an exit code.
155+
156+
``storage_factory`` builds the ``StorageBase`` the run caches weather and tariff
157+
downloads through, and is called as ``storage_factory(work_dir, log)`` - exactly how
158+
``StorageLocalFiles`` is constructed, so the default is that class itself and the
159+
command line behaves as it always has.
160+
161+
It exists because ``StorageBase`` is already an abstraction (``annual_weather`` and
162+
``annual_tariff`` only ever call ``self.storage.load``/``save``), but this entry point
163+
hard-coded the one implementation, so the only way to run the annual tool against a
164+
different backend was to fork the CLI. A caller embedding the tool in a long-lived
165+
service - where a per-process work dir means every process re-downloads the same
166+
immutable ERA5 and Octopus data, and nothing can be shared between them - can now pass
167+
a factory for their own backend and reuse everything else here, including the
168+
``--machine`` stdout/stderr contract, which is the fiddly part to reimplement.
169+
"""
155170
parser = argparse.ArgumentParser(description="Project a year of electricity costs using the Predbat engine")
156171
parser.add_argument("--config", required=True, help="Path to the annual prediction YAML config")
157172
parser.add_argument("--out", default=None, help="Write the results JSON to this path")
@@ -174,7 +189,7 @@ def main(argv=None):
174189
# process depends on. The default (non-machine) path keeps log=print unchanged.
175190
log = _stderr_log if args.machine else print
176191

177-
storage = StorageLocalFiles(args.work_dir, log)
192+
storage = storage_factory(args.work_dir, log)
178193

179194
# predictor.run() lazily imports the full Predbat engine (predbat.py) on its first call
180195
# to create_headless_predbat(); that module's top-level self-update check

apps/predbat/tests/test_annual_cli.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import annual_cli
1818
from annual_cli import format_table, make_progress
19+
from storage import StorageLocalFiles
1920

2021

2122
class _StubPredictor:
@@ -27,10 +28,12 @@ class _StubPredictor:
2728
"""
2829

2930
captured_log = None
31+
captured_storage = None
3032

3133
def __init__(self, config, log=None, storage=None, work_dir=None):
32-
"""Record the log callable and discard everything else."""
34+
"""Record the log callable and the storage, and discard everything else."""
3335
_StubPredictor.captured_log = log
36+
_StubPredictor.captured_storage = storage
3437

3538
async def run(self, progress=None):
3639
"""Report one fake progress step (if asked) and return canned results."""
@@ -237,6 +240,59 @@ def test_annual_cli(my_predbat):
237240
print(" ERROR: --quiet should still construct AnnualPredictor with log=print, got {}".format(_StubPredictor.captured_log))
238241
failed = True
239242

243+
print("Test: storage_factory defaults to local files and is called with (work_dir, log)")
244+
# The default must keep the command line behaving exactly as it did before the factory
245+
# existed, so a plain run is still backed by StorageLocalFiles rooted at --work-dir.
246+
original_predictor = annual_cli.AnnualPredictor
247+
annual_cli.AnnualPredictor = _StubPredictor
248+
with tempfile.TemporaryDirectory() as work_dir:
249+
config_path = os.path.join(work_dir, "annual.yaml")
250+
with open(config_path, "w") as handle:
251+
handle.write("annual: {}\n")
252+
try:
253+
with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()):
254+
annual_cli.main(["--config", config_path, "--work-dir", os.path.join(work_dir, "work")])
255+
finally:
256+
annual_cli.AnnualPredictor = original_predictor
257+
258+
if not isinstance(_StubPredictor.captured_storage, StorageLocalFiles):
259+
print(" ERROR: the default storage should be StorageLocalFiles, got {!r}".format(_StubPredictor.captured_storage))
260+
failed = True
261+
262+
print("Test: a supplied storage_factory replaces it, receiving the work dir and log")
263+
captured_args = {}
264+
265+
class _StubStorage:
266+
"""Records what main() hands the factory, standing in for a non-file backend."""
267+
268+
def _factory(work_dir, log):
269+
captured_args["work_dir"] = work_dir
270+
captured_args["log"] = log
271+
return _StubStorage()
272+
273+
original_predictor = annual_cli.AnnualPredictor
274+
annual_cli.AnnualPredictor = _StubPredictor
275+
with tempfile.TemporaryDirectory() as work_dir:
276+
config_path = os.path.join(work_dir, "annual.yaml")
277+
with open(config_path, "w") as handle:
278+
handle.write("annual: {}\n")
279+
expected_work = os.path.join(work_dir, "work")
280+
try:
281+
with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()):
282+
annual_cli.main(["--config", config_path, "--work-dir", expected_work], storage_factory=_factory)
283+
finally:
284+
annual_cli.AnnualPredictor = original_predictor
285+
286+
if not isinstance(_StubPredictor.captured_storage, _StubStorage):
287+
print(" ERROR: the supplied factory's storage should reach AnnualPredictor, got {!r}".format(_StubPredictor.captured_storage))
288+
failed = True
289+
if captured_args.get("work_dir") != expected_work:
290+
print(" ERROR: the factory should receive --work-dir, got {!r}".format(captured_args.get("work_dir")))
291+
failed = True
292+
if captured_args.get("log") is not print:
293+
print(" ERROR: the factory should receive the run's log callable, got {!r}".format(captured_args.get("log")))
294+
failed = True
295+
240296
return failed
241297

242298

0 commit comments

Comments
 (0)