Skip to content

Commit fc1642f

Browse files
committed
style: trim redundant comments and docstrings
Audit pass against the repo's "comments only when the WHY is non-obvious" standard. Deleted three test docstrings that restated the function name. Shortened five over-explanatory comments to one line each. Kept the assertion-failure diagnostics that genuinely help the next maintainer (sushi config drift messages; the kwarg-presence assertion message documenting why we don't only check the value). Net: -38 lines / +6 lines across the touched files. No behaviour change. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com>
1 parent 8016f2a commit fc1642f

5 files changed

Lines changed: 6 additions & 38 deletions

File tree

sqlmesh/cli/main.py

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

118118
load = True
119-
# Computed outside the single-path block: local-only behaviour must apply
120-
# regardless of how many --paths were provided, unlike SKIP_LOAD_COMMANDS.
119+
# Outside the single-path block: applies regardless of --paths count.
121120
load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS
122121

123122
if len(paths) == 1:

sqlmesh/core/context.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ class GenericContext(BaseContext, t.Generic[C]):
363363
connection as it appears in configuration will be used.
364364
concurrent_tasks: The maximum number of tasks that can use the connection concurrently.
365365
load: Whether or not to automatically load all models and macros (default True).
366-
load_state: Whether to merge remote state into the local project during load (default True). Consulted by load(); has no effect when load() is not called.
366+
load_state: Whether to merge remote state into the local project during load (default True).
367367
console: The rich instance used for printing out CLI command results.
368368
users: A list of users to make known to SQLMesh.
369369
"""

tests/cli/test_cli.py

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,7 +2240,6 @@ def test_format_leading_comma_default(runner: CliRunner, tmp_path: Path):
22402240

22412241

22422242
def _setup_local_only_project(tmp_path, mocker):
2243-
"""Scaffold a project with a non-empty `project` name and patch state to raise."""
22442243
create_example_project(tmp_path, template=ProjectTemplate.EMPTY)
22452244
config_path = tmp_path / "config.yaml"
22462245
existing = config_path.read_text(encoding="utf-8")
@@ -2272,22 +2271,7 @@ def test_lint_runs_without_state(runner: CliRunner, tmp_path: Path, mocker):
22722271

22732272

22742273
def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker):
2275-
"""Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS.
2276-
2277-
Asserts two complementary things, because either alone has a blind spot:
2278-
1. The CLI explicitly passes `load_state=True` to `Context(...)` for `plan`.
2279-
Checking `"load_state" in call.kwargs` (not just the value) catches a
2280-
regression where someone deletes the `load_state=load_state` line in
2281-
cli/main.py entirely — the kwarg would be absent and silently default
2282-
to True without this check.
2283-
2. State sync was actually accessed. Catches a regression where someone
2284-
adds `"plan"` to LOCAL_ONLY_COMMANDS — `load_state` would be False and
2285-
the patched method would never be called.
2286-
2287-
The `plan` invocation is expected to fail because state access is patched
2288-
to raise. We don't assert on exit_code; mocker.spy records the constructor
2289-
call before the wrapped __init__ runs, so kwargs are recorded regardless.
2290-
"""
2274+
"""Guard that `plan` explicitly passes `load_state=True` and still reaches state sync."""
22912275
mock = _setup_local_only_project(tmp_path, mocker)
22922276
init_spy = mocker.spy(Context, "__init__")
22932277

@@ -2307,18 +2291,7 @@ def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker):
23072291
def test_format_does_not_open_state_connection(
23082292
runner: CliRunner, tmp_path: Path, mocker, monkeypatch
23092293
):
2310-
"""Realistic CI scenario: a config.yaml declaring a remote Postgres state
2311-
connection with credentials sourced from unset env vars. Format must still
2312-
succeed without opening the state connection.
2313-
2314-
Distinct from test_format_runs_without_state: that one proves the gate by
2315-
patching get_versions on a default-DuckDB project. This one proves the
2316-
end-to-end CI use case where the config declares a real remote backend and
2317-
no secrets are provisioned. Jinja `env_var('FOO')` with `FOO` unset
2318-
renders the string `"None"` into the YAML; Pydantic accepts that for
2319-
Postgres's `user`/`password` (both typed `str`), so config validation
2320-
passes with placeholder values. The gate then prevents any connection.
2321-
"""
2294+
"""Format must not open a configured remote Postgres state connection when CI secrets are unset."""
23222295
pytest.importorskip("psycopg2")
23232296

23242297
for var in ("PG_HOST", "PG_USER", "PG_PASSWORD", "PG_DATABASE"):

tests/core/linter/test_builtin.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -235,15 +235,13 @@ 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 built-in rules without touching state sync."""
239238
sushi_paths = copy_to_temp_path("examples/sushi")
240239
sushi_path = sushi_paths[0]
241240

242241
with open(sushi_path / "config.py", "r") as f:
243242
read_file = f.read()
244243

245-
# Set a non-empty project name so `any(self._projects)` is truthy and the
246-
# state-merge guard in `Context.load()` actually exercises `self._load_state`.
244+
# Set a project name so state-merge code reaches the `self._load_state` guard.
247245
project_anchor = "config = Config(\n gateways="
248246
assert project_anchor in read_file, (
249247
"sushi config.py shape drifted; update project_anchor in test"
@@ -253,8 +251,7 @@ def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None:
253251
'config = Config(\n project="sushi",\n gateways=',
254252
)
255253

256-
# Enable the linter with one built-in rule so `lint_models` actually executes
257-
# a rule under `load_state=False`, not just the empty-rule-set path.
254+
# Enable one built-in rule so `lint_models` doesn't take the empty-rule-set path.
258255
before = """ linter=LinterConfig(
259256
enabled=False,
260257
rules=[

tests/core/test_format.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ def test_ignore_formating_files(tmp_path: pathlib.Path):
147147

148148

149149
def test_format_without_state_load(tmp_path: pathlib.Path, mocker: MockerFixture):
150-
"""`format` with `load_state=False` runs end-to-end without touching state sync."""
151150
mock = mocker.patch(
152151
"sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions",
153152
side_effect=RuntimeError("state should not be accessed"),

0 commit comments

Comments
 (0)