Dikootje/dbt-core
1
0
Fork 0
forked from repo-mirrors/dbt-core
Code Pull Requests Activity

Target path should be relative to project dir, rather than current working directory (#7706)

Browse Source
This commit is contained in:
Gerda Shank
2023-05-26 18:50:38 -04:00
committed by GitHub
parent 7e3a6eec96
commit 38ca4fce25
19 changed files with 89 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
.changes/unreleased/Fixes-20230525-165053.yaml Normal file
View File

@@ -0,0 +1,6 @@
kind: Fixes
body: Incorrect paths used for "target" and "state" directories
time: 2023-05-25T16:50:53.718564-04:00
custom:
Author: gshank
Issue: "7465"

4
core/dbt/cli/params.py
View File

@@ -431,7 +431,7 @@ state = click.option(
dir_okay=True,
file_okay=False,
readable=True,
resolve_path=True,
resolve_path=False,
path_type=Path,
),
)
@@ -444,7 +444,7 @@ defer_state = click.option(
dir_okay=True,
file_okay=False,
readable=True,
resolve_path=True,
resolve_path=False,
path_type=Path,
),
)

2
core/dbt/cli/requires.py
View File

@@ -247,7 +247,7 @@ def manifest(*args0, write=True, write_perf_info=False):
ctx.obj["manifest"] = manifest
if write and ctx.obj["flags"].write_json:
write_manifest(manifest, ctx.obj["runtime_config"].target_path)
write_manifest(manifest, ctx.obj["runtime_config"].project_target_path)
return func(*args, **kwargs)

13
core/dbt/compilation.py
View File

@@ -272,7 +272,7 @@ class Compiler:
self.config = config
def initialize(self):
make_directory(self.config.target_path)
make_directory(self.config.project_target_path)
make_directory(self.config.packages_install_path)
# creates a ModelContext which is converted to
@@ -512,7 +512,9 @@ class Compiler:
# including the test edges.
summaries["with_test_edges"] = linker.get_graph_summary(manifest)
with open(os.path.join(self.config.target_path, "graph_summary.json"), "w") as out_stream:
with open(
os.path.join(self.config.project_target_path, "graph_summary.json"), "w"
) as out_stream:
try:
out_stream.write(json.dumps(summaries))
except Exception as e: # This is non-essential information, so merely note failures.
@@ -539,7 +541,7 @@ class Compiler:
def write_graph_file(self, linker: Linker, manifest: Manifest):
filename = graph_file_name
graph_path = os.path.join(self.config.target_path, filename)
graph_path = os.path.join(self.config.project_target_path, filename)
flags = get_flags()
if flags.WRITE_JSON:
linker.write_graph(graph_path, manifest)
@@ -554,9 +556,8 @@ class Compiler:
fire_event(WritingInjectedSQLForNode(node_info=get_node_info()))
if node.compiled_code:
node.compiled_path = node.write_node(
self.config.target_path, "compiled", node.compiled_code
)
node.compiled_path = node.get_target_write_path(self.config.target_path, "compiled")
node.write_node(self.config.project_root, node.compiled_path, node.compiled_code)
return node
def compile_node(

5
core/dbt/config/project.py
View File

@@ -700,3 +700,8 @@ class Project:
if dispatch_entry["macro_namespace"] == macro_namespace:
return dispatch_entry["search_order"]
return None
@property
def project_target_path(self):
# If target_path is absolute, project_root will not be included
return os.path.join(self.project_root, self.target_path)

3
core/dbt/context/providers.py
View File

@@ -833,7 +833,8 @@ class ProviderContext(ManifestContext):
# macros/source defs aren't 'writeable'.
if isinstance(self.model, (Macro, SourceDefinition)):
raise MacrosSourcesUnWriteableError(node=self.model)
self.model.build_path = self.model.write_node(self.config.target_path, "run", payload)
self.model.build_path = self.model.get_target_write_path(self.config.target_path, "run")
self.model.write_node(self.config.project_root, self.model.build_path, payload)
return ""
@contextmember

14
core/dbt/contracts/graph/nodes.py
View File

@@ -345,17 +345,23 @@ class ParsedNode(NodeInfoMixin, ParsedNodeMandatory, SerializableType):
relation_name: Optional[str] = None
raw_code: str = ""
def write_node(self, target_path: str, subdirectory: str, payload: str):
def get_target_write_path(self, target_path: str, subdirectory: str):
# This is called for both the "compiled" subdirectory of "target" and the "run" subdirectory
if os.path.basename(self.path) == os.path.basename(self.original_file_path):
# One-to-one relationship of nodes to files.
path = self.original_file_path
else:
# Many-to-one relationship of nodes to files.
path = os.path.join(self.original_file_path, self.path)
full_path = os.path.join(target_path, subdirectory, self.package_name, path)
target_write_path = os.path.join(target_path, subdirectory, self.package_name, path)
return target_write_path
write_file(full_path, payload)
return full_path
def write_node(self, project_root: str, compiled_path, compiled_code: str):
if os.path.isabs(compiled_path):
full_path = compiled_path
else:
full_path = os.path.join(project_root, compiled_path)
write_file(full_path, compiled_code)
def _serialize(self):
return self.to_dict()

16
core/dbt/contracts/state.py
View File

@@ -7,15 +7,17 @@ from dbt.exceptions import IncompatibleSchemaError
class PreviousState:
def __init__(self, path: Path, current_path: Path):
self.path: Path = path
self.current_path: Path = current_path
def __init__(self, state_path: Path, target_path: Path, project_root: Path):
self.state_path: Path = state_path
self.target_path: Path = target_path
self.project_root: Path = project_root
self.manifest: Optional[WritableManifest] = None
self.results: Optional[RunResultsArtifact] = None
self.sources: Optional[FreshnessExecutionResultArtifact] = None
self.sources_current: Optional[FreshnessExecutionResultArtifact] = None
manifest_path = self.path / "manifest.json"
# Note: if state_path is absolute, project_root will be ignored.
manifest_path = self.project_root / self.state_path / "manifest.json"
if manifest_path.exists() and manifest_path.is_file():
try:
self.manifest = WritableManifest.read_and_check_versions(str(manifest_path))
@@ -23,7 +25,7 @@ class PreviousState:
exc.add_filename(str(manifest_path))
raise
results_path = self.path / "run_results.json"
results_path = self.project_root / self.state_path / "run_results.json"
if results_path.exists() and results_path.is_file():
try:
self.results = RunResultsArtifact.read_and_check_versions(str(results_path))
@@ -31,7 +33,7 @@ class PreviousState:
exc.add_filename(str(results_path))
raise
sources_path = self.path / "sources.json"
sources_path = self.project_root / self.state_path / "sources.json"
if sources_path.exists() and sources_path.is_file():
try:
self.sources = FreshnessExecutionResultArtifact.read_and_check_versions(
@@ -41,7 +43,7 @@ class PreviousState:
exc.add_filename(str(sources_path))
raise
sources_current_path = self.current_path / "sources.json"
sources_current_path = self.project_root / self.target_path / "sources.json"
if sources_current_path.exists() and sources_current_path.is_file():
try:
self.sources_current = FreshnessExecutionResultArtifact.read_and_check_versions(

10
core/dbt/parser/manifest.py
View File

@@ -326,7 +326,7 @@ class ManifestLoader:
loader.track_project_load()
if write_perf_info:
loader.write_perf_info(config.target_path)
loader.write_perf_info(config.project_target_path)
return manifest
@@ -729,9 +729,7 @@ class ManifestLoader:
macro.depends_on.add_macro(dep_macro_id) # will check for dupes
def write_manifest_for_partial_parse(self):
path = os.path.join(
self.root_project.project_root, self.root_project.target_path, PARTIAL_PARSE_FILE_NAME
)
path = os.path.join(self.root_project.project_target_path, PARTIAL_PARSE_FILE_NAME)
try:
# This shouldn't be necessary, but we have gotten bug reports (#3757) of the
# saved manifest not matching the code version.
@@ -944,9 +942,7 @@ class ManifestLoader:
if not get_flags().PARTIAL_PARSE:
fire_event(PartialParsingNotEnabled())
return None
path = os.path.join(
self.root_project.project_root, self.root_project.target_path, PARTIAL_PARSE_FILE_NAME
)
path = os.path.join(self.root_project.project_target_path, PARTIAL_PARSE_FILE_NAME)
reparse_reason = None

2
core/dbt/task/compile.py
View File

@@ -125,7 +125,7 @@ class CompileTask(GraphRunnableTask):
favor_state=bool(self.args.favor_state),
)
# TODO: is it wrong to write the manifest here? I think it's right...
write_manifest(self.manifest, self.config.target_path)
write_manifest(self.manifest, self.config.project_target_path)
def _runtime_initialize(self):
if getattr(self.args, "inline", None):

2
core/dbt/task/freshness.py
View File

@@ -159,7 +159,7 @@ class FreshnessTask(GraphRunnableTask):
if self.args.output:
return os.path.realpath(self.args.output)
else:
return os.path.join(self.config.target_path, RESULT_FILE_NAME)
return os.path.join(self.config.project_target_path, RESULT_FILE_NAME)
def raise_on_first_error(self):
return False

10
core/dbt/task/generate.py
View File

@@ -214,10 +214,12 @@ class GenerateTask(CompileTask):
compile_results=compile_results,
)
shutil.copyfile(DOCS_INDEX_FILE_PATH, os.path.join(self.config.target_path, "index.html"))
shutil.copyfile(
DOCS_INDEX_FILE_PATH, os.path.join(self.config.project_target_path, "index.html")
)
for asset_path in self.config.asset_paths:
to_asset_path = os.path.join(self.config.target_path, asset_path)
to_asset_path = os.path.join(self.config.project_target_path, asset_path)
if os.path.exists(to_asset_path):
shutil.rmtree(to_asset_path)
@@ -257,10 +259,10 @@ class GenerateTask(CompileTask):
errors=errors,
)
path = os.path.join(self.config.target_path, CATALOG_FILENAME)
path = os.path.join(self.config.project_target_path, CATALOG_FILENAME)
results.write(path)
if self.args.compile:
write_manifest(self.manifest, self.config.target_path)
write_manifest(self.manifest, self.config.project_target_path)
if exceptions:
fire_event(WriteCatalogFailure(num_exceptions=len(exceptions)))

2
core/dbt/task/run_operation.py
View File

@@ -101,7 +101,7 @@ class RunOperationTask(ConfiguredTask):
results=[run_result],
)
result_path = os.path.join(self.config.target_path, RESULT_FILE_NAME)
result_path = os.path.join(self.config.project_target_path, RESULT_FILE_NAME)
if self.args.write_json:
results.write(result_path)

12
core/dbt/task/runnable.py
View File

@@ -77,12 +77,16 @@ class GraphRunnableTask(ConfiguredTask):
if self.args.state:
self.previous_state = PreviousState(
path=self.args.state, current_path=Path(self.config.target_path)
state_path=self.args.state,
target_path=Path(self.config.target_path),
project_root=Path(self.config.project_root),
)
if self.args.defer_state:
self.previous_defer_state = PreviousState(
path=self.args.defer_state, current_path=Path(self.config.target_path)
state_path=self.args.defer_state,
target_path=Path(self.config.target_path),
project_root=Path(self.config.project_root),
)
def index_offset(self, value: int) -> int:
@@ -158,7 +162,7 @@ class GraphRunnableTask(ConfiguredTask):
raise NotImplementedError("Not Implemented")
def result_path(self):
return os.path.join(self.config.target_path, RESULT_FILE_NAME)
return os.path.join(self.config.project_target_path, RESULT_FILE_NAME)
def get_runner(self, node):
adapter = get_adapter(self.config)
@@ -457,7 +461,7 @@ class GraphRunnableTask(ConfiguredTask):
)
if self.args.write_json:
write_manifest(self.manifest, self.config.target_path)
write_manifest(self.manifest, self.config.project_target_path)
if hasattr(result, "write"):
result.write(self.result_path())

2
core/dbt/task/serve.py
View File

@@ -12,7 +12,7 @@ from dbt.task.base import ConfiguredTask
class ServeTask(ConfiguredTask):
def run(self):
os.chdir(self.config.target_path)
os.chdir(self.config.project_target_path)
shutil.copyfile(DOCS_INDEX_FILE_PATH, "index.html")
port = self.args.port

4
test/unit/test_graph_selector_methods.py
View File

@@ -1202,7 +1202,9 @@ def test_select_metric(manifest):
def previous_state(manifest):
writable = copy.deepcopy(manifest).writable_manifest()
state = PreviousState(
path=Path("/path/does/not/exist"), current_path=Path("/path/does/not/exist")
state_path=Path("/path/does/not/exist"),
target_path=Path("/path/does/not/exist"),
project_root=Path("/path/does/not/exist"),
)
state.manifest = writable
return state

6
tests/functional/configs/test_configs.py
View File

@@ -58,11 +58,13 @@ class TestTargetConfigs(BaseConfigProject):
}
def test_alternative_target_paths(self, project):
# chdir to a different directory to test creation of target directory under project_root
os.chdir(project.profiles_dir)
run_dbt(["seed"])
target_path = ""
for d in os.listdir("."):
if os.path.isdir(d) and d.startswith("target_"):
for d in os.listdir(project.project_root):
if os.path.isdir(os.path.join(project.project_root, d)) and d.startswith("target_"):
target_path = d
assert os.path.exists(os.path.join(project.project_root, target_path, "manifest.json"))

32
tests/functional/defer_state/test_defer_state.py
View File

@@ -79,12 +79,15 @@ class BaseDeferState:
outputs["otherschema"]["schema"] = other_schema
return {"test": {"outputs": outputs, "target": "default"}}
def copy_state(self):
if not os.path.exists("state"):
os.makedirs("state")
shutil.copyfile("target/manifest.json", "state/manifest.json")
def copy_state(self, project_root):
state_path = os.path.join(project_root, "state")
if not os.path.exists(state_path):
os.makedirs(state_path)
shutil.copyfile(
f"{project_root}/target/manifest.json", f"{project_root}/state/manifest.json"
)
def run_and_save_state(self):
def run_and_save_state(self, project_root):
results = run_dbt(["seed"])
assert len(results) == 1
assert not any(r.node.deferred for r in results)
@@ -95,7 +98,7 @@ class BaseDeferState:
assert len(results) == 2
# copy files
self.copy_state()
self.copy_state(project_root)
class TestDeferStateUnsupportedCommands(BaseDeferState):
@@ -112,9 +115,12 @@ class TestDeferStateUnsupportedCommands(BaseDeferState):
class TestRunCompileState(BaseDeferState):
def test_run_and_compile_defer(self, project):
self.run_and_save_state()
self.run_and_save_state(project.project_root)
# defer test, it succeeds
# Change directory to ensure that state directory is underneath
# project directory.
os.chdir(project.profiles_dir)
results = run_dbt(["compile", "--state", "state", "--defer"])
assert len(results.results) == 6
assert results.results[0].node.name == "seed"
@@ -122,11 +128,11 @@ class TestRunCompileState(BaseDeferState):
class TestSnapshotState(BaseDeferState):
def test_snapshot_state_defer(self, project):
self.run_and_save_state()
self.run_and_save_state(project.project_root)
# snapshot succeeds without --defer
run_dbt(["snapshot"])
# copy files
self.copy_state()
self.copy_state(project.project_root)
# defer test, it succeeds
run_dbt(["snapshot", "--state", "state", "--defer"])
# favor_state test, it succeeds
@@ -136,7 +142,7 @@ class TestSnapshotState(BaseDeferState):
class TestRunDeferState(BaseDeferState):
def test_run_and_defer(self, project, unique_schema, other_schema):
project.create_test_schema(other_schema)
self.run_and_save_state()
self.run_and_save_state(project.project_root)
# test tests first, because run will change things
# no state, wrong schema, failure.
@@ -188,7 +194,7 @@ class TestRunDeferState(BaseDeferState):
class TestRunDeferStateChangedModel(BaseDeferState):
def test_run_defer_state_changed_model(self, project):
self.run_and_save_state()
self.run_and_save_state(project.project_root)
# change "view_model"
write_file(changed_view_model_sql, "models", "view_model.sql")
@@ -217,7 +223,7 @@ class TestRunDeferStateChangedModel(BaseDeferState):
class TestRunDeferStateIFFNotExists(BaseDeferState):
def test_run_defer_iff_not_exists(self, project, unique_schema, other_schema):
project.create_test_schema(other_schema)
self.run_and_save_state()
self.run_and_save_state(project.project_root)
results = run_dbt(["seed", "--target", "otherschema"])
assert len(results) == 1
@@ -240,7 +246,7 @@ class TestRunDeferStateIFFNotExists(BaseDeferState):
class TestDeferStateDeletedUpstream(BaseDeferState):
def test_run_defer_deleted_upstream(self, project, unique_schema, other_schema):
project.create_test_schema(other_schema)
self.run_and_save_state()
self.run_and_save_state(project.project_root)
# remove "ephemeral_model" + change "table_model"
rm_file("models", "ephemeral_model.sql")

4
tests/functional/multi_project/test_publication.py
View File

@@ -1,6 +1,5 @@
import json
import pytest
import os
from dbt.tests.util import (
run_dbt,
@@ -241,8 +240,6 @@ class TestMultiProjects:
def test_multi_projects(self, project, project_alt):
# run the alternate project by using the alternate project root
# (There is currently a bug where project-dir requires a chdir to work.)
os.chdir(project_alt.project_root)
results, log_output = run_dbt_and_capture(
["--debug", "--log-format=json", "run", "--project-dir", str(project_alt.project_root)]
)
@@ -255,7 +252,6 @@ class TestMultiProjects:
assert len(publication.public_models) == 1
# run the base project
os.chdir(project.project_root)
write_file(dependencies_alt_yml, project.project_root, "dependencies.yml")
results = run_dbt(
["run", "--project-dir", str(project.project_root)], publications=[publication]