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

Fix #8160: Warn when --state == --target #8638

Merged
merged 6 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
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-20230912-225329.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Warn when --state == --target
time: 2023-09-12T22:53:29.869746+01:00
custom:
Author: aranke
Issue: "8160"
12 changes: 9 additions & 3 deletions core/dbt/contracts/state.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from pathlib import Path
from .graph.manifest import WritableManifest
from .results import RunResultsArtifact
from .results import FreshnessExecutionResultArtifact
from typing import Optional

from dbt.contracts.graph.manifest import WritableManifest
from dbt.contracts.results import FreshnessExecutionResultArtifact
from dbt.contracts.results import RunResultsArtifact
from dbt.events.functions import fire_event
from dbt.events.types import WarnStateTargetEqual
from dbt.exceptions import IncompatibleSchemaError


Expand All @@ -16,6 +19,9 @@ def __init__(self, state_path: Path, target_path: Path, project_root: Path):
self.sources: Optional[FreshnessExecutionResultArtifact] = None
self.sources_current: Optional[FreshnessExecutionResultArtifact] = None

if self.state_path == self.target_path:
fire_event(WarnStateTargetEqual(state_path=str(self.state_path)))

# 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():
Expand Down
9 changes: 9 additions & 0 deletions core/dbt/events/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,15 @@ message UnversionedBreakingChangeMsg {
UnversionedBreakingChange data = 2;
}

// I072
message WarnStateTargetEqual {
string state_path = 1;
}

message WarnStateTargetEqualMsg {
EventInfo info = 1;
WarnStateTargetEqual data = 2;
}

// M - Deps generation

Expand Down
14 changes: 12 additions & 2 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import json

from dbt.ui import line_wrap_message, warning_tag, red, green, yellow
from dbt.constants import MAXIMUM_SEED_SIZE_NAME, PIN_PACKAGE_URL
from dbt.events.base_types import (
DynamicLevel,
Expand All @@ -11,8 +10,8 @@
EventLevel,
)
from dbt.events.format import format_fancy_output_line, pluralize, timestamp_to_datetime_string

from dbt.node_types import NodeType
from dbt.ui import line_wrap_message, warning_tag, red, green, yellow


# The classes in this file represent the data necessary to describe a
Expand Down Expand Up @@ -1247,6 +1246,17 @@ def message(self) -> str:
)


class WarnStateTargetEqual(WarnLevel):
def code(self):
return "I072"

def message(self) -> str:
return yellow(
f"Warning: The state and target directories are the same: '{self.state_path}'. "
f"This could lead to missing changes due to overwritten state including non-idempotent retries."
)


# =======================================================
# M - Deps generation
# =======================================================
Expand Down
1,750 changes: 877 additions & 873 deletions core/dbt/events/types_pb2.py

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@

-- as a general rule, data platforms that can clone tables can also do atomic 'create or replace'
{% call statement('main') %}
{{ create_or_replace_clone(target_relation, defer_relation) }}
{% if target_relation and defer_relation and target_relation == defer_relation %}
{{ log("Target relation and defer relation are the same, skipping clone for relation: " ~ target_relation) }}
{% else %}
{{ create_or_replace_clone(target_relation, defer_relation) }}
{% endif %}

{% endcall %}

{% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %}
Expand Down
23 changes: 20 additions & 3 deletions tests/adapter/dbt/tests/adapter/dbt_clone/test_dbt_clone.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import pytest
import os
import shutil
from copy import deepcopy
from collections import Counter
from copy import deepcopy

import pytest

from dbt.exceptions import DbtRuntimeError
from dbt.tests.util import run_dbt
from dbt.tests.adapter.dbt_clone.fixtures import (
seed_csv,
table_model_sql,
Expand All @@ -18,6 +19,7 @@
infinite_macros_sql,
custom_can_clone_tables_false_macros_sql,
)
from dbt.tests.util import run_dbt, run_dbt_and_capture


class BaseClone:
Expand Down Expand Up @@ -215,3 +217,18 @@ def clean_up(self, project):
project.adapter.drop_schema(relation)

pass


class TestCloneSameTargetAndState(BaseClone):
def test_clone_same_target_and_state(self, project, unique_schema, other_schema):
project.create_test_schema(other_schema)
self.run_and_save_state(project.project_root)

clone_args = [
"clone",
"--state",
"target",
]

results, output = run_dbt_and_capture(clone_args, expect_pass=False)
assert "Warning: The state and target directories are the same: 'target'" in output
7 changes: 4 additions & 3 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import pytest
import re
from argparse import Namespace
from typing import TypeVar

import pytest

from dbt.contracts.results import TimingInfo, RunResult, RunStatus
from dbt.events import AdapterLogger, types
from dbt.events.base_types import (
Expand All @@ -19,8 +21,6 @@
from dbt.events.helpers import get_json_string_utcnow
from dbt.events.types import RunResultError
from dbt.flags import set_from_args
from argparse import Namespace

from dbt.task.printer import print_run_result_error

set_from_args(Namespace(WARN_ERROR=False), None)
Expand Down Expand Up @@ -264,6 +264,7 @@ def test_event_codes(self):
enforced_model_constraint_removed=[],
materialization_changed=[],
),
types.WarnStateTargetEqual(state_path=""),
# M - Deps generation ======================
types.GitSparseCheckoutSubdirectory(subdir=""),
types.GitProgressCheckoutRevision(revision=""),
Expand Down