From 57aef33fb3fa010a08fc10b2fec9db3c6347a1f7 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Thu, 9 Feb 2023 12:21:39 -0800 Subject: [PATCH] Click cli main merge (#6926) --- .../unreleased/Fixes-20230123-132814.yaml | 6 + .../unreleased/Fixes-20230207-143544.yaml | 6 + .../unreleased/Fixes-20230208-110551.yaml | 6 + .github/workflows/release-branch-tests.yml | 30 ++++- core/dbt/events/types.proto | 16 +++ core/dbt/lib.py | 2 + core/dbt/main.py | 13 ++ core/dbt/task/parse.py | 0 core/dbt/task/run.py | 2 +- .../test_incremental_merge_exclude_columns.py | 116 ++++++++++++++++++ tests/functional/hooks/fixtures.py | 28 +++++ tests/functional/hooks/test_run_hooks.py | 24 ++++ 12 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 .changes/unreleased/Fixes-20230123-132814.yaml create mode 100644 .changes/unreleased/Fixes-20230207-143544.yaml create mode 100644 .changes/unreleased/Fixes-20230208-110551.yaml create mode 100644 core/dbt/task/parse.py create mode 100644 tests/adapter/dbt/tests/adapter/incremental/test_incremental_merge_exclude_columns.py diff --git a/.changes/unreleased/Fixes-20230123-132814.yaml b/.changes/unreleased/Fixes-20230123-132814.yaml new file mode 100644 index 00000000000..f05bac4571a --- /dev/null +++ b/.changes/unreleased/Fixes-20230123-132814.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: add merge_exclude_columns adapter tests +time: 2023-01-23T13:28:14.808748-06:00 +custom: + Author: dave-connors-3 + Issue: "6699" diff --git a/.changes/unreleased/Fixes-20230207-143544.yaml b/.changes/unreleased/Fixes-20230207-143544.yaml new file mode 100644 index 00000000000..67850c91927 --- /dev/null +++ b/.changes/unreleased/Fixes-20230207-143544.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix regression of --quiet cli parameter behavior +time: 2023-02-07T14:35:44.160163-05:00 +custom: + Author: peterallenwebb + Issue: "6749" diff --git a/.changes/unreleased/Fixes-20230208-110551.yaml b/.changes/unreleased/Fixes-20230208-110551.yaml new file mode 100644 index 00000000000..591374a4a33 --- /dev/null +++ b/.changes/unreleased/Fixes-20230208-110551.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Ensure results from hooks contain nodes when processing them +time: 2023-02-08T11:05:51.952494-06:00 +custom: + Author: emmyoop + Issue: "6796" diff --git a/.github/workflows/release-branch-tests.yml b/.github/workflows/release-branch-tests.yml index bdd01aa495a..b31a7c8c3b1 100644 --- a/.github/workflows/release-branch-tests.yml +++ b/.github/workflows/release-branch-tests.yml @@ -28,7 +28,33 @@ on: permissions: read-all jobs: + fetch-latest-branches: + runs-on: ubuntu-latest + + outputs: + latest-branches: ${{ steps.get-latest-branches.outputs.repo-branches }} + + steps: + - name: "Fetch dbt-core Latest Branches" + uses: dbt-labs/actions/fetch-repo-branches@v1.1.1 + id: get-latest-branches + with: + repo_name: ${{ github.event.repository.name }} + organization: "dbt-labs" + pat: ${{ secrets.GITHUB_TOKEN }} + fetch_protected_branches_only: true + regex: "^1.[0-9]+.latest$" + perform_match_method: "match" + retries: 3 + + - name: "[ANNOTATION] ${{ github.event.repository.name }} - branches to test" + run: | + title="${{ github.event.repository.name }} - branches to test" + message="The workflow will run tests for the following branches of the ${{ github.event.repository.name }} repo: ${{ steps.get-latest-branches.outputs.repo-branches }}" + echo "::notice $title::$message" + kick-off-ci: + needs: [fetch-latest-branches] name: Kick-off CI runs-on: ubuntu-latest @@ -39,7 +65,9 @@ jobs: max-parallel: 1 fail-fast: false matrix: - branch: [1.0.latest, 1.1.latest, 1.2.latest, 1.3.latest, 1.4.latest, main] + branch: ${{ fromJSON(needs.fetch-latest-branches.outputs.latest-branches) }} + include: + - branch: 'main' steps: - name: Call CI workflow for ${{ matrix.branch }} branch diff --git a/core/dbt/events/types.proto b/core/dbt/events/types.proto index ed57077e1da..23418fa7179 100644 --- a/core/dbt/events/types.proto +++ b/core/dbt/events/types.proto @@ -1556,6 +1556,13 @@ message LogSeedResultMsg { // Skipped Q017 +message LogSeedResultMsg { + EventInfo info = 1; + LogSeedResult data = 2; +} + +// Skipped Q017 + // Q018 message LogFreshnessResult { string status = 1; @@ -1719,6 +1726,15 @@ message NothingToDo { } message NothingToDoMsg { + NodeInfo node_info = 1; + string resource_type = 2; + string schema = 3; + string node_name = 4; + int32 index = 5; + int32 total = 6; +} + +message SkippingDetailsMsg { EventInfo info = 1; NothingToDo data = 2; } diff --git a/core/dbt/lib.py b/core/dbt/lib.py index 056b89fa30e..bcd48c5a403 100644 --- a/core/dbt/lib.py +++ b/core/dbt/lib.py @@ -74,6 +74,8 @@ def get_dbt_config(project_dir, args=None, single_threaded=False): profile_name = getattr(args, "profile", None) + profile_name = getattr(args, "profile", None) + runtime_args = RuntimeArgs( project_dir=project_dir, profiles_dir=profiles_dir, diff --git a/core/dbt/main.py b/core/dbt/main.py index 3ea3521868e..cb86f6ffc2b 100644 --- a/core/dbt/main.py +++ b/core/dbt/main.py @@ -1024,6 +1024,19 @@ def parse_args(args, cls=DBTArgumentParser): """, ) + warn_error_flag.add_argument( + "--warn-error-options", + default=None, + help=""" + If dbt would normally warn, instead raise an exception based on + include/exclude configuration. Examples include --select that selects + nothing, deprecations, configurations with no associated models, + invalid test configurations, and missing sources/refs in tests. + This argument should be a YAML string, with keys 'include' or 'exclude'. + eg. '{"include": "all", "exclude": ["NoNodesForSelectionCriteria"]}' + """, + ) + p.add_argument( "--no-version-check", dest="version_check", diff --git a/core/dbt/task/parse.py b/core/dbt/task/parse.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/core/dbt/task/run.py b/core/dbt/task/run.py index 2c251c2aff6..213627a4651 100644 --- a/core/dbt/task/run.py +++ b/core/dbt/task/run.py @@ -443,7 +443,7 @@ def after_run(self, adapter, results): database_schema_set: Set[Tuple[Optional[str], str]] = { (r.node.database, r.node.schema) for r in results - if r.node.is_relational + if (hasattr(r, "node") and r.node.is_relational) and r.status not in (NodeStatus.Error, NodeStatus.Fail, NodeStatus.Skipped) } diff --git a/tests/adapter/dbt/tests/adapter/incremental/test_incremental_merge_exclude_columns.py b/tests/adapter/dbt/tests/adapter/incremental/test_incremental_merge_exclude_columns.py new file mode 100644 index 00000000000..db958f1eda4 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/incremental/test_incremental_merge_exclude_columns.py @@ -0,0 +1,116 @@ +import pytest +from dbt.tests.util import run_dbt, check_relations_equal +from collections import namedtuple + + +models__merge_exclude_columns_sql = """ +{{ config( + materialized = 'incremental', + unique_key = 'id', + incremental_strategy='merge', + merge_exclude_columns=['msg'] +) }} + +{% if not is_incremental() %} + +-- data for first invocation of model + +select 1 as id, 'hello' as msg, 'blue' as color +union all +select 2 as id, 'goodbye' as msg, 'red' as color + +{% else %} + +-- data for subsequent incremental update + +select 1 as id, 'hey' as msg, 'blue' as color +union all +select 2 as id, 'yo' as msg, 'green' as color +union all +select 3 as id, 'anyway' as msg, 'purple' as color + +{% endif %} +""" + +seeds__expected_merge_exclude_columns_csv = """id,msg,color +1,hello,blue +2,goodbye,green +3,anyway,purple +""" + +ResultHolder = namedtuple( + "ResultHolder", + [ + "seed_count", + "model_count", + "seed_rows", + "inc_test_model_count", + "relation", + ], +) + + +class BaseMergeExcludeColumns: + @pytest.fixture(scope="class") + def models(self): + return {"merge_exclude_columns.sql": models__merge_exclude_columns_sql} + + @pytest.fixture(scope="class") + def seeds(self): + return {"expected_merge_exclude_columns.csv": seeds__expected_merge_exclude_columns_csv} + + def update_incremental_model(self, incremental_model): + """update incremental model after the seed table has been updated""" + model_result_set = run_dbt(["run", "--select", incremental_model]) + return len(model_result_set) + + def get_test_fields(self, project, seed, incremental_model, update_sql_file): + + seed_count = len(run_dbt(["seed", "--select", seed, "--full-refresh"])) + + model_count = len(run_dbt(["run", "--select", incremental_model, "--full-refresh"])) + + relation = incremental_model + # update seed in anticipation of incremental model update + row_count_query = "select * from {}.{}".format(project.test_schema, seed) + + seed_rows = len(project.run_sql(row_count_query, fetch="all")) + + # propagate seed state to incremental model according to unique keys + inc_test_model_count = self.update_incremental_model(incremental_model=incremental_model) + + return ResultHolder(seed_count, model_count, seed_rows, inc_test_model_count, relation) + + def check_scenario_correctness(self, expected_fields, test_case_fields, project): + """Invoke assertions to verify correct build functionality""" + # 1. test seed(s) should build afresh + assert expected_fields.seed_count == test_case_fields.seed_count + # 2. test model(s) should build afresh + assert expected_fields.model_count == test_case_fields.model_count + # 3. seeds should have intended row counts post update + assert expected_fields.seed_rows == test_case_fields.seed_rows + # 4. incremental test model(s) should be updated + assert expected_fields.inc_test_model_count == test_case_fields.inc_test_model_count + # 5. result table should match intended result set (itself a relation) + check_relations_equal( + project.adapter, [expected_fields.relation, test_case_fields.relation] + ) + + def test__merge_exclude_columns(self, project): + """seed should match model after two incremental runs""" + + expected_fields = ResultHolder( + seed_count=1, + model_count=1, + inc_test_model_count=1, + seed_rows=3, + relation="expected_merge_exclude_columns", + ) + + test_case_fields = self.get_test_fields( + project, + seed="expected_merge_exclude_columns", + incremental_model="merge_exclude_columns", + update_sql_file=None, + ) + self.check_scenario_correctness(expected_fields, test_case_fields, project) diff --git a/tests/functional/hooks/fixtures.py b/tests/functional/hooks/fixtures.py index 6a721fffea5..36f6eaa350a 100644 --- a/tests/functional/hooks/fixtures.py +++ b/tests/functional/hooks/fixtures.py @@ -1,3 +1,31 @@ +macros_missing_column = """ +{% macro export_table_check() %} + + {% set table = 'test_column' %} + + {% set query %} + SELECT column_name + FROM {{ref(table)}} + LIMIT 1 + {% endset %} + + {%- if flags.WHICH in ('run', 'build') -%} + {% set results = run_query(query) %} + {% if execute %} + {%- if results.rows -%} + {{ exceptions.raise_compiler_error("ON_RUN_START_CHECK_NOT_PASSED: Data already exported. DBT Run aborted.") }} + {% else -%} + {{ log("No data found in " ~ table ~ " for current day and runtime region. Proceeding...", true) }} + {%- endif -%} + {%- endif -%} + {%- endif -%} +{% endmacro %} +""" + +models__missing_column = """ +select 1 as col +""" + macros__before_and_after = """ {% macro custom_run_hook(state, target, run_started_at, invocation_id) %} diff --git a/tests/functional/hooks/test_run_hooks.py b/tests/functional/hooks/test_run_hooks.py index ddda7473fc1..cc285198523 100644 --- a/tests/functional/hooks/test_run_hooks.py +++ b/tests/functional/hooks/test_run_hooks.py @@ -8,6 +8,8 @@ macros__before_and_after, models__hooks, seeds__example_seed_csv, + macros_missing_column, + models__missing_column, ) from dbt.tests.util import ( @@ -141,3 +143,25 @@ def test_pre_and_post_seed_hooks(self, setUp, project, dbt_profile_target): check_table_does_not_exist(project.adapter, "start_hook_order_test") check_table_does_not_exist(project.adapter, "end_hook_order_test") self.assert_used_schemas(project) + + +class TestAfterRunHooks(object): + @pytest.fixture(scope="class") + def macros(self): + return {"temp_macro.sql": macros_missing_column} + + @pytest.fixture(scope="class") + def models(self): + return {"test_column.sql": models__missing_column} + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + # The create and drop table statements here validate that these hooks run + # in the same order that they are defined. Drop before create is an error. + # Also check that the table does not exist below. + "on-run-start": "- {{ export_table_check() }}" + } + + def test_missing_column_pre_hook(self, project): + run_dbt(["run"], expect_pass=False)