Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
May 26, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20230525-165053.yaml
Original file line number Diff line number Diff line change
@@ -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 changes: 2 additions & 2 deletions core/dbt/cli/params.py
Original file line number Diff line number Diff line change
@@ -431,7 +431,7 @@
dir_okay=True,
file_okay=False,
readable=True,
resolve_path=True,
resolve_path=False,
path_type=Path,
),
)
@@ -444,7 +444,7 @@
dir_okay=True,
file_okay=False,
readable=True,
resolve_path=True,
resolve_path=False,
path_type=Path,
),
)
2 changes: 1 addition & 1 deletion core/dbt/cli/requires.py
Original file line number Diff line number Diff line change
@@ -247,7 +247,7 @@ def wrapper(*args, **kwargs):

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 changes: 7 additions & 6 deletions core/dbt/compilation.py
Original file line number Diff line number Diff line change
@@ -272,7 +272,7 @@ def __init__(self, config):
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 @@ def compile(self, manifest: Manifest, write=True, add_test_edges=False) -> Graph
# 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 @@ def compile(self, manifest: Manifest, write=True, add_test_edges=False) -> Graph

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 @@ def _write_node(self, node: ManifestSQLNode) -> ManifestSQLNode:
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 changes: 5 additions & 0 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
@@ -700,3 +700,8 @@ def get_macro_search_order(self, macro_namespace: str):
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 changes: 2 additions & 1 deletion core/dbt/context/providers.py
Original file line number Diff line number Diff line change
@@ -833,7 +833,8 @@ def write(self, payload: str) -> str:
# 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 changes: 10 additions & 4 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
@@ -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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we convert this to a custom context manager, so we can centralize logic in one place and use with statements?

https://realpython.com/python-with-statement/#creating-custom-context-managers

# 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 changes: 9 additions & 7 deletions core/dbt/contracts/state.py
Original file line number Diff line number Diff line change
@@ -7,31 +7,33 @@


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))
except IncompatibleSchemaError as exc:
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))
except IncompatibleSchemaError as exc:
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 @@ def __init__(self, path: Path, current_path: Path):
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 changes: 3 additions & 7 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
@@ -326,7 +326,7 @@ def get_full_manifest(
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 @@ def macro_depends_on(self):
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 @@ def read_manifest_for_partial_parse(self) -> Optional[Manifest]:
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 changes: 1 addition & 1 deletion core/dbt/task/compile.py
Original file line number Diff line number Diff line change
@@ -125,7 +125,7 @@ def defer_to_manifest(self, adapter, selected_uids: AbstractSet[str]):
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 changes: 1 addition & 1 deletion core/dbt/task/freshness.py
Original file line number Diff line number Diff line change
@@ -159,7 +159,7 @@ def result_path(self):
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 changes: 6 additions & 4 deletions core/dbt/task/generate.py
Original file line number Diff line number Diff line change
@@ -214,10 +214,12 @@ def run(self) -> CatalogArtifact:
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 @@ def run(self) -> CatalogArtifact:
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 changes: 1 addition & 1 deletion core/dbt/task/run_operation.py
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ def run(self) -> RunResultsArtifact:
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 changes: 8 additions & 4 deletions core/dbt/task/runnable.py
Original file line number Diff line number Diff line change
@@ -77,12 +77,16 @@ def __init__(self, args, config, manifest):

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 @@ def get_runner_type(self, node):
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 @@ def run(self):
)

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 changes: 1 addition & 1 deletion core/dbt/task/serve.py
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@

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 changes: 3 additions & 1 deletion test/unit/test_graph_selector_methods.py
Original file line number Diff line number Diff line change
@@ -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 changes: 4 additions & 2 deletions tests/functional/configs/test_configs.py
Original file line number Diff line number Diff line change
@@ -58,11 +58,13 @@ def project_config_update(self):
}

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"))

Loading