Skip to content

Commit ea59785

Browse files
committed
fix: address whole-PR review findings
Three issues caught in the final review against the spec: * Multi-path CLI invocations were still loading state. The LOCAL_ONLY_COMMANDS check was inside `if len(paths) == 1:`, mirroring the existing `SKIP_LOAD_COMMANDS` shape. But unlike load-skipping, local-only behaviour is independent of project count - multi-project monorepo users running `sqlmesh --paths a --paths b format` should not hit state either. Lifted the check out of the conditional. Verified empirically: multi-path format against an unreachable Postgres state now exits 0. * `GenericContext.__init__` had `load_state` inserted between `load` and `users`, shifting positional arguments for any caller outside this repo passing `users`, `config_loader_kwargs`, or `selector` positionally. Moved `load_state` to the end of the signature. In-repo callers (cli/main.py, magics.py, github controller) all use kwargs so they are unaffected. * The lint test was running with the linter disabled, so no built-in rule actually executed under `load_state=False`. Added the existing pattern's `LinterConfig(enabled=False, ...)` -> `enabled=True, rules=["nomissingexternalmodels"]` rewrite so the test exercises a real rule's code path while state is patched to raise. Plan updated to reflect the final signature placement and the lifted CLI check. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com>
1 parent addfc19 commit ea59785

4 files changed

Lines changed: 22 additions & 7 deletions

File tree

docs/2026-05-21_local-only-format/plan.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ Two tasks, two commits, one PR.
9595

9696
In `sqlmesh/core/context.py`:
9797
- Extend the `GenericContext` class docstring args block at lines 356-368 with a one-line entry for the new parameter: a brief description noting it gates the remote-state merge inside `load()` and is only meaningful when `load=True`.
98-
- Add `load_state: bool = True` to `GenericContext.__init__` after the existing `load: bool = True` at line 385.
98+
- Add `load_state: bool = True` to `GenericContext.__init__` at the *end* of the parameter list (after `selector`). Placing it last avoids shifting any existing positional arguments for callers outside this repo who may pass `users`, `config_loader_kwargs`, or `selector` positionally.
9999
- Store as `self._load_state` on the instance during `__init__`, alongside the other private attributes (`self._loaded`, `self._loaders`, etc., around lines 396-415).
100100
- In `Context.load()`, tighten the two `if any(self._projects):` guards at lines 677 and 685 by ANDing `self._load_state` into each predicate. No other changes to `load()` body. `update_schemas`, `Linter.from_rules`, and the `analytics.collector.on_project_loaded` call are unaffected.
101101

@@ -135,7 +135,7 @@ All three tests share a setup factored into `_setup_local_only_project(tmp_path,
135135

136136
In `sqlmesh/cli/main.py`:
137137
- Add a new module-level constant `LOCAL_ONLY_COMMANDS = ("format", "lint")` immediately after `SKIP_CONTEXT_COMMANDS` (line 43), matching the surrounding tuple style.
138-
- Inside `cli(...)` (lines 117-123), mirror the existing `load = True; if ... in SKIP_LOAD_COMMANDS: load = False` pattern to introduce a `load_state` local that defaults to `True` and flips to `False` when `ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS`.
138+
- Inside `cli(...)` (around line 117), compute `load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS` *outside* the `if len(paths) == 1:` block. Unlike `SKIP_LOAD_COMMANDS` (whose `load = False` toggle is inside the single-path conditional), `load_state` must apply regardless of how many `--paths` were provided — multi-project monorepo invocations of `format`/`lint` need to be local-only too.
139139
- Pass `load_state=load_state` as an additional keyword argument in the `Context(...)` call at lines 132-137.
140140

141141
**Verification:**

sqlmesh/cli/main.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def cli(
116116
configure_console(ignore_warnings=ignore_warnings)
117117

118118
load = True
119-
load_state = True
119+
load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS
120120

121121
if len(paths) == 1:
122122
path = os.path.abspath(paths[0])
@@ -125,8 +125,6 @@ def cli(
125125
return
126126
if ctx.invoked_subcommand in SKIP_LOAD_COMMANDS:
127127
load = False
128-
if ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS:
129-
load_state = False
130128

131129
configs = load_configs(config, Context.CONFIG_TYPE, paths, dotenv_path=dotenv)
132130
log_limit = list(configs.values())[0].log_limit

sqlmesh/core/context.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,10 +384,10 @@ def __init__(
384384
concurrent_tasks: t.Optional[int] = None,
385385
loader: t.Optional[t.Type[Loader]] = None,
386386
load: bool = True,
387-
load_state: bool = True,
388387
users: t.Optional[t.List[User]] = None,
389388
config_loader_kwargs: t.Optional[t.Dict[str, t.Any]] = None,
390389
selector: t.Optional[t.Type[Selector]] = None,
390+
load_state: bool = True,
391391
):
392392
self.configs = (
393393
config

tests/core/linter/test_builtin.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def test_no_missing_unit_tests(tmp_path, copy_to_temp_path):
235235

236236

237237
def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None:
238-
"""`lint_models` with `load_state=False` runs end-to-end without touching state sync."""
238+
"""`lint_models` with `load_state=False` runs built-in rules without touching state sync."""
239239
sushi_paths = copy_to_temp_path("examples/sushi")
240240
sushi_path = sushi_paths[0]
241241

@@ -250,6 +250,23 @@ def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None:
250250
)
251251
assert 'project="sushi"' in read_file
252252

253+
# Enable the linter with one built-in rule so `lint_models` actually executes
254+
# a rule under `load_state=False`, not just the empty-rule-set path.
255+
before = """ linter=LinterConfig(
256+
enabled=False,
257+
rules=[
258+
"ambiguousorinvalidcolumn",
259+
"invalidselectstarexpansion",
260+
"noselectstar",
261+
"nomissingaudits",
262+
"nomissingowner",
263+
"nomissingexternalmodels",
264+
],
265+
),"""
266+
after = 'linter=LinterConfig(enabled=True, rules=["nomissingexternalmodels"]),'
267+
read_file = read_file.replace(before, after)
268+
assert after in read_file
269+
253270
with open(sushi_path / "config.py", "w") as f:
254271
f.writelines(read_file)
255272

0 commit comments

Comments
 (0)