Skip to content

Commit 8cf8c62

Browse files
committed
feat(format): skip the project load when paths are given
`sqlmesh format` already accepted positional paths and never loaded state, but it still loaded every model in the project and then filtered them down with `Path.samefile`. That load is wasted work: formatting pretty-prints a file's own text, and needs nothing from the rest of the project graph. Formatting a file needs only three things — the file's text, the config of the project that owns its path, and the dialect and formatting flag. The first two come from `config_for_path`, which resolves a config from a path alone, and the last two are read off the file's own MODEL/AUDIT header. So when paths are given the project is no longer loaded, and `format` now joins `lint` in scoping its own load: with no paths it loads the project itself, which also leaves the web and LSP callers untouched since those contexts are already loaded. Whether a file is a model is now decided by parsing its header rather than by membership in the loaded project, which keeps macros and other non-model SQL a no-op and ignores Python model paths. One consequence worth calling out: a standalone audit is formatted when selected by path, while the project-wide pass still skips it, because it is loaded into `_standalone_audits` rather than `_audits`. That gap predates this change and is noted on the issue. Collecting targets through a small named tuple lets both the path-based and project-wide routes share one format-and-write loop, and drops the `samefile` call that ran once per model in the project. Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
1 parent ee57c61 commit 8cf8c62

5 files changed

Lines changed: 339 additions & 41 deletions

File tree

‎docs/reference/cli.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,14 @@ Options:
224224
## format
225225

226226
```
227-
Usage: sqlmesh format [OPTIONS]
227+
Usage: sqlmesh format [OPTIONS] [PATHS]...
228228
229229
Format all SQL models and audits.
230230
231+
PATHS are SQL model or audit files. When given, only those files are
232+
formatted and the project is not loaded. Paths that are not SQL models or
233+
audits, such as macros, are left alone.
234+
231235
Options:
232236
-t, --transpile TEXT Transpile project models to the specified
233237
dialect.

‎sqlmesh/cli/main.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,9 @@ def cli(
141141
if ctx.invoked_subcommand in SKIP_LOAD_COMMANDS:
142142
load = False
143143

144-
# Unlike the other commands above, lint can scope its own load for multi-project contexts.
145-
if ctx.invoked_subcommand == "lint":
144+
# Unlike the other commands above, lint and format scope their own load: lint for
145+
# multi-project contexts, format because it needs nothing loaded when given file paths.
146+
if ctx.invoked_subcommand in ("lint", "format"):
146147
load = False
147148

148149
configs = load_configs(config, Context.CONFIG_TYPE, paths, dotenv_path=dotenv)
@@ -395,7 +396,12 @@ def evaluate(
395396
def format(
396397
ctx: click.Context, paths: t.Optional[t.Tuple[str, ...]] = None, **kwargs: t.Any
397398
) -> None:
398-
"""Format all SQL models and audits."""
399+
"""Format all SQL models and audits.
400+
401+
PATHS are SQL model or audit files. When given, only those files are formatted and the
402+
project is not loaded. Paths that are not SQL models or audits, such as macros, are left
403+
alone.
404+
"""
399405
if not ctx.obj.format(**{k: v for k, v in kwargs.items() if v is not None}, paths=paths):
400406
ctx.exit(1)
401407

‎sqlmesh/core/context.py‎

Lines changed: 122 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
from datetime import datetime
5050

5151
from sqlglot import Dialect, exp
52-
from sqlglot.helper import first
52+
from sqlglot.helper import first, seq_get
5353
from sqlglot.lineage import GraphHTML
5454

5555
from sqlmesh.core import analytics
@@ -67,6 +67,8 @@
6767
from sqlmesh.core.console import get_console
6868
from sqlmesh.core.context_diff import ContextDiff
6969
from sqlmesh.core.dialect import (
70+
Audit as AuditMeta,
71+
Model as ModelMeta,
7072
format_model_expressions,
7173
is_meta_expression,
7274
normalize_model_name,
@@ -119,7 +121,7 @@
119121
filter_tests_by_patterns,
120122
)
121123
from sqlmesh.core.user import User
122-
from sqlmesh.utils import CorrelationId, UniqueKeyDict, Verbosity
124+
from sqlmesh.utils import CorrelationId, UniqueKeyDict, Verbosity, str_to_bool
123125
from sqlmesh.utils.concurrency import concurrent_apply_to_values
124126
from sqlmesh.utils.dag import DAG
125127
from sqlmesh.utils.date import (
@@ -164,6 +166,16 @@
164166
logger = logging.getLogger(__name__)
165167

166168

169+
class FormatTarget(t.NamedTuple):
170+
"""A single file to format, with everything needed to format it already resolved."""
171+
172+
path: Path
173+
config: Config
174+
dialect: t.Optional[str]
175+
before: str
176+
expressions: t.List[exp.Expr]
177+
178+
167179
class BaseContext(abc.ABC):
168180
"""The base context which defines methods to execute a model."""
169181

@@ -1323,41 +1335,38 @@ def format(
13231335
paths: t.Optional[t.Tuple[t.Union[str, Path], ...]] = None,
13241336
**kwargs: t.Any,
13251337
) -> bool:
1326-
"""Format all SQL models and audits."""
1327-
filtered_targets = [
1328-
target
1329-
for target in chain(self._models.values(), self._audits.values())
1330-
if target._path is not None
1331-
and target._path.suffix == ".sql"
1332-
and (not paths or any(target._path.samefile(p) for p in paths))
1333-
]
1334-
unformatted_file_paths = []
1338+
"""Format SQL models and audits.
13351339
1336-
for target in filtered_targets:
1337-
if (
1338-
target._path is None or target.formatting is False
1339-
): # introduced to satisfy type checker as still want to pull filter out as many targets as possible before loop
1340-
continue
1340+
Args:
1341+
paths: Files to format. When given, the project is not loaded: formatting a file
1342+
only needs its own text, its project's config, and the dialect and formatting
1343+
flag off its MODEL/AUDIT header. Without them, every SQL model and audit in the
1344+
project is formatted, loading it first if necessary.
1345+
"""
1346+
if paths:
1347+
targets = self._format_targets_for_paths(paths)
1348+
else:
1349+
if not self._loaded:
1350+
self.load()
1351+
targets = self._format_targets_for_project()
13411352

1342-
mode = "r" if check else "r+"
1343-
with open(target._path, mode, encoding="utf-8") as file:
1344-
before = file.read()
1353+
unformatted_file_paths = []
13451354

1346-
after = self._format(
1347-
target,
1348-
before,
1349-
transpile=transpile,
1350-
rewrite_casts=rewrite_casts,
1351-
append_newline=append_newline,
1352-
**kwargs,
1353-
)
1355+
for path, config, dialect, before, expressions in targets:
1356+
after = self._format_expressions(
1357+
expressions,
1358+
config=config,
1359+
dialect=dialect,
1360+
transpile=transpile,
1361+
rewrite_casts=rewrite_casts,
1362+
append_newline=append_newline,
1363+
**kwargs,
1364+
)
13541365

1355-
if not check:
1356-
file.seek(0)
1357-
file.write(after)
1358-
file.truncate()
1359-
elif before != after:
1360-
unformatted_file_paths.append(target._path)
1366+
if not check:
1367+
path.write_text(after, encoding="utf-8")
1368+
elif before != after:
1369+
unformatted_file_paths.append(path)
13611370

13621371
if unformatted_file_paths:
13631372
for path in unformatted_file_paths:
@@ -1369,31 +1378,107 @@ def format(
13691378

13701379
return True
13711380

1381+
def _format_targets_for_project(self) -> t.Iterator[FormatTarget]:
1382+
"""Every SQL model and audit in the loaded project."""
1383+
for target in chain(self._models.values(), self._audits.values()):
1384+
if target._path is None or target._path.suffix != ".sql":
1385+
continue
1386+
if target.formatting is False:
1387+
continue
1388+
1389+
config = self.config_for_node(target)
1390+
before = target._path.read_text(encoding="utf-8")
1391+
yield FormatTarget(
1392+
path=target._path,
1393+
config=config,
1394+
dialect=target.dialect,
1395+
before=before,
1396+
expressions=parse(before, default_dialect=config.dialect),
1397+
)
1398+
1399+
def _format_targets_for_paths(
1400+
self, paths: t.Tuple[t.Union[str, Path], ...]
1401+
) -> t.Iterator[FormatTarget]:
1402+
"""The given files, resolved without loading the project.
1403+
1404+
The dialect and the formatting flag are read off the file's own MODEL/AUDIT header,
1405+
falling back to the model defaults of the config that owns the path. Anything that is
1406+
not a SQL model or audit file is skipped, which leaves macros and other SQL alone.
1407+
"""
1408+
for path in (Path(p) for p in paths):
1409+
if path.suffix != ".sql":
1410+
continue
1411+
1412+
config = self.config_for_path(path)[0]
1413+
before = path.read_text(encoding="utf-8")
1414+
expressions = parse(before, default_dialect=config.dialect)
1415+
1416+
meta = seq_get(expressions, 0)
1417+
if not isinstance(meta, (ModelMeta, AuditMeta)):
1418+
continue
1419+
1420+
properties = {prop.name.lower(): prop.args.get("value") for prop in meta.expressions}
1421+
1422+
formatting = properties.get("formatting")
1423+
if isinstance(formatting, exp.Boolean):
1424+
formatting = formatting.this
1425+
else:
1426+
# Model defaults are not passed through a model's bool validator here, so a
1427+
# configured string has to be coerced the same way that validator would.
1428+
formatting = config.model_defaults.formatting
1429+
if isinstance(formatting, str):
1430+
formatting = str_to_bool(formatting)
1431+
if formatting is False:
1432+
continue
1433+
1434+
dialect = properties.get("dialect")
1435+
yield FormatTarget(
1436+
path=path,
1437+
config=config,
1438+
dialect=dialect.name if isinstance(dialect, exp.Literal) else config.dialect,
1439+
before=before,
1440+
expressions=expressions,
1441+
)
1442+
13721443
def _format(
13731444
self,
13741445
target: Model | Audit,
13751446
before: str,
1447+
**kwargs: t.Any,
1448+
) -> str:
1449+
config = self.config_for_node(target)
1450+
return self._format_expressions(
1451+
parse(before, default_dialect=config.dialect),
1452+
config=config,
1453+
dialect=target.dialect,
1454+
**kwargs,
1455+
)
1456+
1457+
def _format_expressions(
1458+
self,
1459+
expressions: t.List[exp.Expr],
13761460
*,
1461+
config: Config,
1462+
dialect: t.Optional[str],
13771463
transpile: t.Optional[str] = None,
13781464
rewrite_casts: t.Optional[bool] = None,
13791465
append_newline: t.Optional[bool] = None,
13801466
**kwargs: t.Any,
13811467
) -> str:
1382-
expressions = parse(before, default_dialect=self.config_for_node(target).dialect)
13831468
if transpile and is_meta_expression(expressions[0]):
13841469
for prop in expressions[0].expressions:
13851470
if prop.name.lower() == "dialect":
13861471
prop.replace(
13871472
exp.Property(
13881473
this="dialect",
1389-
value=exp.Literal.string(transpile or target.dialect),
1474+
value=exp.Literal.string(transpile or dialect),
13901475
)
13911476
)
13921477

1393-
format_config = self.config_for_node(target).format
1478+
format_config = config.format
13941479
after = format_model_expressions(
13951480
expressions,
1396-
transpile or target.dialect,
1481+
transpile or dialect,
13971482
rewrite_casts=(
13981483
rewrite_casts if rewrite_casts is not None else not format_config.no_rewrite_casts
13991484
),

‎tests/cli/test_cli.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2611,3 +2611,27 @@ def test_format_does_not_open_state_connection(
26112611
result = runner.invoke(cli, ["--paths", str(tmp_path), "format"])
26122612
assert result.exit_code == 0, f"Format failed: {result.output}\nException: {result.exception}"
26132613
mock.assert_not_called()
2614+
2615+
2616+
def test_format_with_paths_skips_project_load(runner: CliRunner, tmp_path: Path, mocker) -> None:
2617+
"""`sqlmesh format <path>` must not load the project."""
2618+
create_example_project(tmp_path)
2619+
load_spy = mocker.spy(Context, "load")
2620+
2621+
model = tmp_path / "models" / "full_model.sql"
2622+
result = runner.invoke(cli, ["--paths", str(tmp_path), "format", str(model)])
2623+
2624+
assert result.exit_code == 0, f"Format failed: {result.output}\nException: {result.exception}"
2625+
load_spy.assert_not_called()
2626+
2627+
2628+
def test_format_without_paths_still_loads_project(
2629+
runner: CliRunner, tmp_path: Path, mocker
2630+
) -> None:
2631+
create_example_project(tmp_path)
2632+
load_spy = mocker.spy(Context, "load")
2633+
2634+
result = runner.invoke(cli, ["--paths", str(tmp_path), "format"])
2635+
2636+
assert result.exit_code == 0, f"Format failed: {result.output}\nException: {result.exception}"
2637+
assert load_spy.called

0 commit comments

Comments
 (0)