Skip to content

Commit 1770e6a

Browse files
committed
Add dp lint command
1 parent d1e91dd commit 1770e6a

File tree

8 files changed

+341
-4
lines changed

8 files changed

+341
-4
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [Unreleased]
44

5+
### Added
6+
7+
- `dp lint` command that uses [SQLFluff](https://github.com/sqlfluff/sqlfluff) to lint models and tests
8+
59
## [0.14.0] - 2022-02-02
610

711
## [0.13.0] - 2022-02-01

data_pipelines_cli/cli.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from .cli_commands.create import create_command
88
from .cli_commands.deploy import deploy_command
99
from .cli_commands.init import init_command
10+
from .cli_commands.lint import lint_command
1011
from .cli_commands.prepare_env import prepare_env_command
1112
from .cli_commands.publish import publish_command
1213
from .cli_commands.run import run_command
@@ -36,6 +37,7 @@ def cli() -> None:
3637
_cli.add_command(create_command)
3738
_cli.add_command(deploy_command)
3839
_cli.add_command(init_command)
40+
_cli.add_command(lint_command)
3941
_cli.add_command(prepare_env_command)
4042
_cli.add_command(publish_command)
4143
_cli.add_command(run_command)
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
import pathlib
2+
import tempfile
3+
from configparser import ConfigParser
4+
from typing import List
5+
6+
import click
7+
import yaml
8+
9+
from ..cli_constants import BUILD_DIR
10+
from ..cli_utils import echo_info, echo_subinfo, echo_warning, subprocess_run
11+
from ..config_generation import (
12+
generate_profiles_yml,
13+
read_dictionary_from_config_directory,
14+
)
15+
from ..errors import SQLLintError, SubprocessNonZeroExitError
16+
17+
SQLFLUFF_FIX_NOT_EVERYTHING_ERROR = 1
18+
SQLFLUFF_LINT_ERROR = 65 # according to `sqlfluff.core.linter.LintingResult.stats`
19+
SQLFLUFF_DIALECT_LOADING_ERROR = 66 # according to `sqlfluff.cli.commands.get_config`
20+
21+
22+
def _get_dialect_or_default() -> str:
23+
"""Read ``dbt.yml`` config file and return its ``target_type`` or just the ``ansi``."""
24+
env, dbt_filename = "base", "dbt.yml"
25+
dbt_env_config = read_dictionary_from_config_directory(
26+
BUILD_DIR.joinpath("dag"), env, dbt_filename
27+
) or read_dictionary_from_config_directory(pathlib.Path.cwd(), env, dbt_filename)
28+
try:
29+
dialect = dbt_env_config["target_type"]
30+
echo_subinfo(f'Found target_type "{dialect}", attempting to use it as the SQL dialect.')
31+
except KeyError:
32+
dialect = "ansi"
33+
echo_warning(
34+
'Could not find `target_type` in `dbt.yml`. Using the default SQL dialect ("ansi").'
35+
)
36+
return dialect
37+
38+
39+
def _get_source_tests_paths() -> List[pathlib.Path]:
40+
with open(pathlib.Path.cwd().joinpath("dbt_project.yml"), "r") as f:
41+
dbt_project_config = yaml.safe_load(f)
42+
dir_names: List[str] = (
43+
dbt_project_config.get("source-paths", [])
44+
+ dbt_project_config.get("model-paths", [])
45+
+ dbt_project_config.get("test-paths", [])
46+
)
47+
return list(map(lambda dir_name: pathlib.Path.cwd().joinpath(dir_name), dir_names))
48+
49+
50+
def _create_temporary_sqlfluff_config(env: str) -> ConfigParser:
51+
config = ConfigParser()
52+
config["sqlfluff"] = {"templater": "dbt"}
53+
config["sqlfluff:templater:dbt"] = {
54+
"profiles_dir": str(generate_profiles_yml(env, copy_config_dir=True).absolute())
55+
}
56+
return config
57+
58+
59+
def _run_sqlfluff(command: str, dialect: str, env: str, additional_args: List[str]) -> None:
60+
with tempfile.TemporaryDirectory() as tmp_dir:
61+
tmp_config_path = pathlib.Path(tmp_dir).joinpath("sqlfluff.config")
62+
with open(tmp_config_path, "w") as tmp_config:
63+
_create_temporary_sqlfluff_config(env).write(tmp_config)
64+
65+
def sqlfluff_args(sql_dialect: str) -> List[str]:
66+
return [
67+
"sqlfluff",
68+
command,
69+
"--dialect",
70+
sql_dialect,
71+
"--config",
72+
str(tmp_config_path),
73+
*additional_args,
74+
*map(str, _get_source_tests_paths()),
75+
]
76+
77+
try:
78+
subprocess_run(sqlfluff_args(dialect))
79+
except SubprocessNonZeroExitError as err:
80+
if err.exit_code == SQLFLUFF_DIALECT_LOADING_ERROR and dialect != "ansi":
81+
subprocess_run(sqlfluff_args("ansi"))
82+
else:
83+
raise err
84+
85+
86+
def _run_fix_sqlfluff(dialect: str, env: str) -> None:
87+
try:
88+
echo_subinfo("Attempting to fix SQLs. Not every error can be automatically fixed.")
89+
_run_sqlfluff("fix", dialect, env, ["--force"])
90+
except SubprocessNonZeroExitError as err:
91+
if err.exit_code != SQLFLUFF_FIX_NOT_EVERYTHING_ERROR:
92+
raise err
93+
94+
95+
def _run_lint_sqlfluff(dialect: str, env: str) -> None:
96+
try:
97+
echo_subinfo("Linting SQLs.")
98+
_run_sqlfluff("lint", dialect, env, [])
99+
except SubprocessNonZeroExitError as err:
100+
if err.exit_code == SQLFLUFF_LINT_ERROR:
101+
raise SQLLintError
102+
else:
103+
raise err
104+
105+
106+
def lint(fix: bool, env: str) -> None:
107+
"""
108+
Lint and format SQL.
109+
110+
:param fix: Whether to lint and fix linting errors, or just lint.
111+
:type fix: bool
112+
:param env: Name of the environment
113+
:type env: str
114+
"""
115+
echo_info("Linting SQLs:")
116+
dialect = _get_dialect_or_default()
117+
if fix:
118+
_run_fix_sqlfluff(dialect, env)
119+
_run_lint_sqlfluff(dialect, env)
120+
121+
122+
@click.command(
123+
name="lint",
124+
short_help="Lint and format SQL",
125+
help="Lint and format SQL using SQLFluff.\n\n"
126+
"For more information on rules and the workings of SQLFluff, "
127+
"refer to https://docs.sqlfluff.com/",
128+
)
129+
@click.option(
130+
"--no-fix",
131+
is_flag=True,
132+
default=False,
133+
type=bool,
134+
help="Whether to lint and fix linting errors, or just lint.",
135+
)
136+
@click.option(
137+
"--env",
138+
default="local",
139+
type=str,
140+
show_default=True,
141+
help="Name of the environment",
142+
)
143+
def lint_command(no_fix: bool, env: str) -> None:
144+
lint(not no_fix, env)

data_pipelines_cli/errors.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@ def __init__(self, project_path: str) -> None:
3838
class SubprocessNonZeroExitError(DataPipelinesError):
3939
"""Exception raised if subprocess exits with non-zero exit code"""
4040

41+
exit_code: int
42+
4143
def __init__(self, subprocess_name: str, exit_code: int) -> None:
4244
self.message = f"{subprocess_name} has exited with non-zero exit code: {exit_code}"
45+
self.exit_code = exit_code
4346

4447

4548
class SubprocessNotFound(DataPipelinesError):
@@ -80,3 +83,10 @@ class DockerErrorResponseError(DataPipelinesError):
8083

8184
def __init__(self, error_msg: str) -> None:
8285
self.message = "Error raised when using Docker.\n" + error_msg
86+
87+
88+
class SQLLintError(DataPipelinesError):
89+
"""Exception raised if there are linting problems in some files."""
90+
91+
def __init__(self) -> None:
92+
self.message = "Fix SQL linting errors."

docs/conf.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import os
1010
import sys
1111

12-
sys.path.insert(0, os.path.abspath("../data_pipelines_cli"))
12+
sys.path.insert(0, os.path.abspath(".."))
1313

1414

1515
# -- Project information -----------------------------------------------------

docs/source/data_pipelines_cli.cli_commands.rst

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ data\_pipelines\_cli.cli\_commands.init module
4949
:undoc-members:
5050
:show-inheritance:
5151

52+
data\_pipelines\_cli.cli\_commands.lint module
53+
----------------------------------------------
54+
55+
.. automodule:: data_pipelines_cli.cli_commands.lint
56+
:members:
57+
:undoc-members:
58+
:show-inheritance:
59+
5260
data\_pipelines\_cli.cli\_commands.prepare\_env module
5361
------------------------------------------------------
5462

@@ -96,4 +104,3 @@ data\_pipelines\_cli.cli\_commands.update module
96104
:members:
97105
:undoc-members:
98106
:show-inheritance:
99-

setup.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010
"questionary==1.10.0",
1111
"pyyaml>=5.1, <6.0",
1212
"types-PyYAML>=6.0",
13-
"copier==5.1.0",
14-
"dbt>=0.21, <0.22",
13+
"dbt==0.21.0",
1514
"Jinja2>=2.11,<2.12",
1615
"fsspec",
16+
"copier==5.1.0",
1717
]
1818

1919
EXTRA_FILESYSTEMS_REQUIRE = {
@@ -25,6 +25,10 @@
2525
"docker": ["docker>=5.0"],
2626
"datahub": ["acryl-datahub>=0.8.17, <0.8.18"],
2727
"git": ["GitPython==3.1.26"],
28+
"lint": [
29+
"sqlfluff==0.9.0",
30+
"sqlfluff-templater-dbt==0.9.0",
31+
],
2832
"tests": [
2933
"pytest>=6.2.2, <7.0.0",
3034
"pytest-cov>=2.8.0, <3.0.0",

0 commit comments

Comments
 (0)