Skip to content
This repository was archived by the owner on May 17, 2024. It is now read-only.

partial --select support for dbt < 1.5 #682

Merged
merged 2 commits into from
Aug 22, 2023
Merged

partial --select support for dbt < 1.5 #682

merged 2 commits into from
Aug 22, 2023

Conversation

dlawin
Copy link
Contributor

@dlawin dlawin commented Aug 15, 2023

This PR allows --select to work with dbt versions < 1.5, but in a much simpler manner

Example on a version < 1.5:
--select <some_input>

The manifest is searched for <some_input> directly regardless of any dbt selectors like +, @ or multiple model names model_1 model_2. In the latter example, it would search the manifest for a single model named model_1 model_2

I've included a warning to help make this clear. See:

Screenshot 2023-08-15 at 1 40 03 PM

@dlawin dlawin added enhancement New feature or request --dbt Issues/features related to the dbt integration labels Aug 15, 2023
@dlawin dlawin self-assigned this Aug 15, 2023
@@ -56,17 +55,18 @@ def test_get_models(self):
mock_self.get_dbt_selection_models.assert_called_once_with(selection)
self.assertEqual(models, mock_return_value)

def test_get_models_unsupported_manifest_version(self):
def test_get_models_simple_select(self):
mock_self = Mock()
mock_self.project_dir = Path()
mock_self.dbt_version = "1.4.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you mock this with these too to capture all the major manifest versions?
https://docs.getdbt.com/reference/artifacts/manifest-json

  • 1.0
  • 1.1
  • 1.2
  • 1.3

Copy link
Contributor

@sungchun12 sungchun12 left a comment

Choose a reason for hiding this comment

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

Really clever implementation! And the warning message nudges the user to upgrade dbt which is so great!

@dlawin
Copy link
Contributor Author

dlawin commented Aug 17, 2023

test_100_fields (tests.test_postgresql.Test100Fields)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/data-diff/data-diff/tests/test_postgresql.py", line 88, in test_100_fields
    self.connection.query('CREATE EXTENSION IF NOT EXISTS "uuid-ossp";', None)
  File "/home/runner/work/data-diff/data-diff/data_diff/sqeleton/databases/base.py", line 368, in query
    res = self._query(sql_code)
  File "/home/runner/work/data-diff/data-diff/data_diff/sqeleton/databases/base.py", line 572, in _query
    return r.result()
  File "/opt/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/concurrent/futures/_base.py", line 446, in result
    return self.__get_result()
  File "/opt/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/concurrent/futures/_base.py", line 3[91](https://github.com/datafold/data-diff/actions/runs/5871224293/job/15924634138?pr=682#step:8:92), in __get_result
    raise self._exception
  File "/opt/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/runner/work/data-diff/data-diff/data_diff/sqeleton/databases/base.py", line 578, in _query_in_worker
    return self._query_conn(self.thread_local.conn, sql_code)
  File "/home/runner/work/data-diff/data-diff/data_diff/sqeleton/databases/base.py", line 530, in _query_conn
    return apply_query(callback, sql_code)
  File "/home/runner/work/data-diff/data-diff/data_diff/sqeleton/databases/base.py", line 117, in apply_query
    return callback(sql_code)
  File "/home/runner/work/data-diff/data-diff/data_diff/sqeleton/databases/base.py", line 518, in _query_cursor
    c.execute(sql_code)
psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "pg_extension_name_index"
DETAIL:  Key (extname)=(uuid-ossp) already exists.

This failure seems unrelated but IF NOT EXISTS isn't being honored... race condition?

@sungchun12
Copy link
Contributor

@dlawin Don't know about the test enough to provide a definite answer but doesn't look like it consistently impacts this PR, so we can ignore it here for now.

@dlawin dlawin merged commit 40e62dc into master Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
--dbt Issues/features related to the dbt integration enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants