From 6f039a8dc24d96abaff6487e90ac351edbbd9d8a Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 14:31:43 -0400 Subject: [PATCH 01/12] Add "project_target_path" property to RuntimeConfig, switch to using it --- core/dbt/cli/requires.py | 2 +- core/dbt/compilation.py | 10 ++++++---- core/dbt/config/project.py | 5 +++++ core/dbt/context/providers.py | 4 +++- core/dbt/parser/manifest.py | 10 +++------- core/dbt/task/compile.py | 2 +- core/dbt/task/freshness.py | 2 +- core/dbt/task/generate.py | 10 ++++++---- core/dbt/task/run_operation.py | 2 +- core/dbt/task/runnable.py | 6 +++--- core/dbt/task/serve.py | 2 +- 11 files changed, 31 insertions(+), 24 deletions(-) diff --git a/core/dbt/cli/requires.py b/core/dbt/cli/requires.py index 5fa2f8c9256..340fc2380fe 100644 --- a/core/dbt/cli/requires.py +++ b/core/dbt/cli/requires.py @@ -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) diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index 12b7a4cc14a..b062c5e7b37 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -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) @@ -555,7 +557,7 @@ def _write_node(self, node: ManifestSQLNode) -> ManifestSQLNode: if node.compiled_code: node.compiled_path = node.write_node( - self.config.target_path, "compiled", node.compiled_code + self.config.project_target_path, "compiled", node.compiled_code ) return node diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 76a3980e26a..28272dd5eb8 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -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): + path = os.path.join(self.project_root, self.target_path) + return path diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index 8a74cc08d1e..a0b7827fdfa 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -833,7 +833,9 @@ 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.write_node( + self.config.project_target_path, "run", payload + ) return "" @contextmember diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 50ce0430a06..414867c96cd 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -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 diff --git a/core/dbt/task/compile.py b/core/dbt/task/compile.py index dbf469cd093..8a5aa2128a1 100644 --- a/core/dbt/task/compile.py +++ b/core/dbt/task/compile.py @@ -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): diff --git a/core/dbt/task/freshness.py b/core/dbt/task/freshness.py index d662e35dd66..32f09dd7470 100644 --- a/core/dbt/task/freshness.py +++ b/core/dbt/task/freshness.py @@ -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 diff --git a/core/dbt/task/generate.py b/core/dbt/task/generate.py index 204d64d7ccd..5e21213e8fb 100644 --- a/core/dbt/task/generate.py +++ b/core/dbt/task/generate.py @@ -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))) diff --git a/core/dbt/task/run_operation.py b/core/dbt/task/run_operation.py index beac272de9a..c614aeda54c 100644 --- a/core/dbt/task/run_operation.py +++ b/core/dbt/task/run_operation.py @@ -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) diff --git a/core/dbt/task/runnable.py b/core/dbt/task/runnable.py index 494acf98904..17bb776cd27 100644 --- a/core/dbt/task/runnable.py +++ b/core/dbt/task/runnable.py @@ -80,7 +80,7 @@ def __init__(self, args, config, manifest): def set_previous_state(self): if self.args.state is not None: self.previous_state = PreviousState( - path=self.args.state, current_path=Path(self.config.target_path) + path=self.args.state, current_path=Path(self.config.project_target_path) ) def index_offset(self, value: int) -> int: @@ -156,7 +156,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) @@ -455,7 +455,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()) diff --git a/core/dbt/task/serve.py b/core/dbt/task/serve.py index 696be89a37f..060c4c93d17 100644 --- a/core/dbt/task/serve.py +++ b/core/dbt/task/serve.py @@ -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 From db8cd1101396d3649b87d703055af8c7f15fc504 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 15:16:07 -0400 Subject: [PATCH 02/12] Split up write_node into "node_compiled_path" and "write_node" functions so that we only store the relative path at node.compiled_path --- core/dbt/compilation.py | 5 ++--- core/dbt/config/project.py | 7 +++++-- core/dbt/contracts/graph/nodes.py | 13 +++++++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index b062c5e7b37..2e363c5d53d 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -556,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.project_target_path, "compiled", node.compiled_code - ) + node.compiled_path = node.get_compiled_path(self.config.target_path, "compiled") + node.write_node(self.config.project_root, node.compiled_path, node.compiled_code) return node def compile_node( diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 28272dd5eb8..08d116fbf3c 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -703,5 +703,8 @@ def get_macro_search_order(self, macro_namespace: str): @property def project_target_path(self): - path = os.path.join(self.project_root, self.target_path) - return path + if os.path.isabs(self.target_path): + return self.target_path + else: + path = os.path.join(self.project_root, self.target_path) + return path diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 4529599b596..782719ff3eb 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -345,17 +345,22 @@ 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_compiled_path(self, target_path: str, subdirectory: str): 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) + compiled_path = os.path.join(target_path, subdirectory, self.package_name, path) + return compiled_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() From 340bb6e3ba32326c0b997f806394b70bb2d39fa7 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 15:55:40 -0400 Subject: [PATCH 03/12] Adjust PreviousState object to use project_root to find "state" and target directories --- core/dbt/contracts/state.py | 16 +++++++++------- core/dbt/task/runnable.py | 4 +++- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/core/dbt/contracts/state.py b/core/dbt/contracts/state.py index cb135e241ac..e6d8d39e735 100644 --- a/core/dbt/contracts/state.py +++ b/core/dbt/contracts/state.py @@ -7,15 +7,17 @@ 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 @@ def __init__(self, path: Path, current_path: Path): 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 @@ def __init__(self, path: Path, current_path: Path): 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.target_path / "sources.json" if sources_current_path.exists() and sources_current_path.is_file(): try: self.sources_current = FreshnessExecutionResultArtifact.read_and_check_versions( diff --git a/core/dbt/task/runnable.py b/core/dbt/task/runnable.py index 17bb776cd27..5560c17551c 100644 --- a/core/dbt/task/runnable.py +++ b/core/dbt/task/runnable.py @@ -80,7 +80,9 @@ def __init__(self, args, config, manifest): def set_previous_state(self): if self.args.state is not None: self.previous_state = PreviousState( - path=self.args.state, current_path=Path(self.config.project_target_path) + state_path=self.args.state, + target_path=Path(self.config.target_path), + project_root=Path(self.config.project_root), ) def index_offset(self, value: int) -> int: From bdc960ab23b62ed8caa3b327929eb088022bccf7 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 16:15:32 -0400 Subject: [PATCH 04/12] Update code to write out the "run" subdirectory of target --- core/dbt/compilation.py | 2 +- core/dbt/context/providers.py | 5 ++--- core/dbt/contracts/graph/nodes.py | 7 ++++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index 2e363c5d53d..c45713b786e 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -556,7 +556,7 @@ def _write_node(self, node: ManifestSQLNode) -> ManifestSQLNode: fire_event(WritingInjectedSQLForNode(node_info=get_node_info())) if node.compiled_code: - node.compiled_path = node.get_compiled_path(self.config.target_path, "compiled") + 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 diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index a0b7827fdfa..fe279a7fd3f 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -833,9 +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.project_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 diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index 782719ff3eb..9d5701c65a6 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -345,15 +345,16 @@ class ParsedNode(NodeInfoMixin, ParsedNodeMandatory, SerializableType): relation_name: Optional[str] = None raw_code: str = "" - def get_compiled_path(self, target_path: str, subdirectory: 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) - compiled_path = os.path.join(target_path, subdirectory, self.package_name, path) - return compiled_path + target_write_path = os.path.join(target_path, subdirectory, self.package_name, path) + return target_write_path def write_node(self, project_root: str, compiled_path, compiled_code: str): if os.path.isabs(compiled_path): From a22fb91ef00630fe4b4fcbaada76a3130fc688d3 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 16:43:07 -0400 Subject: [PATCH 05/12] tweak project_target_path --- core/dbt/config/project.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 08d116fbf3c..96705c7ccb0 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -703,8 +703,5 @@ def get_macro_search_order(self, macro_namespace: str): @property def project_target_path(self): - if os.path.isabs(self.target_path): - return self.target_path - else: - path = os.path.join(self.project_root, self.target_path) - return path + # If target_path is absolute, project_root will not be included + return os.path.join(self.project_root, self.target_path) From 8a9b67b2afdda3f10df251d95be3494d547b5d66 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 16:51:01 -0400 Subject: [PATCH 06/12] Changie --- .changes/unreleased/Fixes-20230525-165053.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20230525-165053.yaml diff --git a/.changes/unreleased/Fixes-20230525-165053.yaml b/.changes/unreleased/Fixes-20230525-165053.yaml new file mode 100644 index 00000000000..89dcd6ddf60 --- /dev/null +++ b/.changes/unreleased/Fixes-20230525-165053.yaml @@ -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" From 7875bb89bc37244eaa09bea0b38e956637e598e5 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 25 May 2023 17:07:09 -0400 Subject: [PATCH 07/12] fix test_graph_selector_methods.py test --- test/unit/test_graph_selector_methods.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/test_graph_selector_methods.py b/test/unit/test_graph_selector_methods.py index 380073fd19f..5bc881747da 100644 --- a/test/unit/test_graph_selector_methods.py +++ b/test/unit/test_graph_selector_methods.py @@ -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 From a5719edb679aaa696665ec0a075e3fe22fb1afa8 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 26 May 2023 09:47:21 -0400 Subject: [PATCH 08/12] Remove chdir that's no longer needed from test --- tests/functional/multi_project/test_publication.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/functional/multi_project/test_publication.py b/tests/functional/multi_project/test_publication.py index 754d02dc73e..c8e3fc6fc9c 100644 --- a/tests/functional/multi_project/test_publication.py +++ b/tests/functional/multi_project/test_publication.py @@ -1,6 +1,5 @@ import json import pytest -import os from dbt.tests.util import ( run_dbt, @@ -241,8 +240,6 @@ def models_alt(self): 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 @@ def test_multi_projects(self, project, project_alt): 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] From e92f3ce6bc79803c0f09c1c424d6c6a164d2b00c Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 26 May 2023 10:09:51 -0400 Subject: [PATCH 09/12] Modify target-path test to run from non-project directory --- tests/functional/configs/test_configs.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/functional/configs/test_configs.py b/tests/functional/configs/test_configs.py index 086ef455f18..49a3222910a 100644 --- a/tests/functional/configs/test_configs.py +++ b/tests/functional/configs/test_configs.py @@ -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")) From 05d298b7624d363138ed64cad6edd6f507a0447e Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 26 May 2023 10:45:25 -0400 Subject: [PATCH 10/12] Remove resolve_path from state click.Path, modify defer_state test to test --- core/dbt/cli/params.py | 2 +- .../defer_state/test_defer_state.py | 32 +++++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/core/dbt/cli/params.py b/core/dbt/cli/params.py index b638fc539dc..39e1ef62141 100644 --- a/core/dbt/cli/params.py +++ b/core/dbt/cli/params.py @@ -431,7 +431,7 @@ dir_okay=True, file_okay=False, readable=True, - resolve_path=True, + resolve_path=False, path_type=Path, ), ) diff --git a/tests/functional/defer_state/test_defer_state.py b/tests/functional/defer_state/test_defer_state.py index a50f09af0d1..43dd6df2fe6 100644 --- a/tests/functional/defer_state/test_defer_state.py +++ b/tests/functional/defer_state/test_defer_state.py @@ -77,12 +77,15 @@ def profiles_config_update(self, dbt_profile_target, unique_schema, other_schema 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) @@ -93,7 +96,7 @@ def run_and_save_state(self): assert len(results) == 2 # copy files - self.copy_state() + self.copy_state(project_root) class TestDeferStateUnsupportedCommands(BaseDeferState): @@ -110,9 +113,12 @@ def test_no_state(self, project): 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" @@ -120,11 +126,11 @@ def test_run_and_compile_defer(self, project): 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 @@ -134,7 +140,7 @@ def test_snapshot_state_defer(self, project): 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. @@ -186,7 +192,7 @@ def test_run_and_defer(self, project, unique_schema, other_schema): 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") @@ -215,7 +221,7 @@ def test_run_defer_state_changed_model(self, project): 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 @@ -238,7 +244,7 @@ def test_run_defer_iff_not_exists(self, project, unique_schema, other_schema): 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") From 366a990284810afbd7a09a15d4f582564aee4a37 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 26 May 2023 16:25:10 -0400 Subject: [PATCH 11/12] use project_root when finding sources.json --- core/dbt/contracts/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/dbt/contracts/state.py b/core/dbt/contracts/state.py index e6d8d39e735..bd9f389b602 100644 --- a/core/dbt/contracts/state.py +++ b/core/dbt/contracts/state.py @@ -43,7 +43,7 @@ def __init__(self, state_path: Path, target_path: Path, project_root: Path): exc.add_filename(str(sources_path)) raise - sources_current_path = self.target_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( From 51a2ebbaac6af9eac0cec768c8eec22332a0c08f Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 26 May 2023 18:00:50 -0400 Subject: [PATCH 12/12] Update to account for defer_state changes --- core/dbt/cli/params.py | 2 +- core/dbt/task/runnable.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/dbt/cli/params.py b/core/dbt/cli/params.py index a80639d95f1..5100ca5dfce 100644 --- a/core/dbt/cli/params.py +++ b/core/dbt/cli/params.py @@ -431,7 +431,7 @@ dir_okay=True, file_okay=False, readable=True, - resolve_path=True, + resolve_path=False, path_type=Path, ), ) diff --git a/core/dbt/task/runnable.py b/core/dbt/task/runnable.py index eda1968f2a7..e3de59df771 100644 --- a/core/dbt/task/runnable.py +++ b/core/dbt/task/runnable.py @@ -84,7 +84,9 @@ def __init__(self, args, config, manifest): 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: