adapters: postgres & redshift #259

cmcarthur · 2016-12-28T16:43:17Z

WIP

drewbanin

This is great -- going to take it for a spin locally

drewbanin · 2017-01-06T14:56:43Z

dbt/exceptions.py

@@ -0,0 +1,2 @@
+class ValidationException(Exception):


drewbanin · 2017-01-06T14:57:59Z

dbt/adapters/postgres.py

+    @classmethod
+    def execute_model(cls, project, target, model):
+        schema_helper = Schema(project, target)
+        parts = re.split(r'-- (DBT_OPERATION .*)', model.compiled_contents)


We're probably going to rethink how DBT_OPERATION works in the future. I think it's fine to leave this how it is for now, but let's definitely refrain from copying this logic into future adapters!

drewbanin · 2017-01-06T14:59:14Z

dbt/main.py

@@ -131,6 +134,7 @@ def parse_args(args):
    p = argparse.ArgumentParser(prog='dbt: data build tool', formatter_class=argparse.RawTextHelpFormatter)
    p.add_argument('--version', action='version', version=dbt.version.get_version_information(), help="Show version information")
    p.add_argument('-d', '--debug', action='store_true', help='Display debug logging during dbt execution. Useful for debugging and making bug reports.')
+    p.add_argument('-S', '--strict', action='store_true', help='Run schema validations at runtime. This will surface bugs in dbt, but may incur a speed penalty.')


drewbanin · 2017-01-06T14:59:35Z

dbt/runner.py

-
-        handle.commit()
-        return status
+        return get_adapter(target).execute_model(


this is so good

…n case of multiple invocations in one python process

drewbanin

I read through this PR... there's a lot going on here and it's looking great to me. I'm going to pull this locally and run it against pg/redshift -- I'm assuming you've tested a good amount against Snowflake. 👍 👍 👍 👍 👍 👍

drewbanin · 2017-01-12T21:11:45Z

Makefile


 test-unit:
 	@echo "Unit test run starting..."
-	tox -e unit-py27,unit-py35
+	@time docker-compose run test tox -e unit-py27,unit-py35,pep8


I've been meaning to talk to you about this. The reason these tests didn't previously use Docker is because we wanted to run CI tests on Windows (via Appveyor). My understanding is that when these tests now run on Appveyor, we're not actually testing if the code works on Windows. Rather, we're testing if it runs in Docker on Windows. Is that what's happening here?

That's not exactly right, since circle & appveyor don't use the Makefile to run tests -- they just call tox directly. The reason to do it this way is so that the tests always run in the same environment during development.

Ok makes sense, thanks

drewbanin · 2017-01-12T21:14:07Z

dbt/adapters/postgres.py

+schema."""
+
+
+@contextmanager


this is super cool

drewbanin · 2017-01-12T21:35:04Z

dbt/adapters/postgres.py

+        update "{schema}"."{table}" set "{tmp_column}" = "{old_column}";
+        alter table "{schema}"."{table}" drop column "{old_column}" cascade;
+        alter table "{schema}"."{table}" rename column "{tmp_column}" to "{old_column}";
+        """.format(**opts).strip()  # noqa


what's noqa

Tells pep8 to skip this line / block

drewbanin · 2017-01-12T21:56:24Z

dbt/adapters/redshift.py

+from dbt.adapters.postgres import PostgresAdapter
+
+
+class RedshiftAdapter(PostgresAdapter):


drewbanin · 2017-01-12T22:34:25Z

dbt/adapters/snowflake.py

+            result['handle'] = handle
+            result['state'] = 'open'
+        except snowflake.connector.errors.Error as e:
+            logger.debug("Got an error when attempting to open a snowflake "


does this fail silently?

That might be the case, yeah. You and I should discuss the ideal functionality here. Related: #253

Ok, this does something sane now:

(hosted-test3) ✔ ~/git/hosted-test [master|✚ 1] 08:31 $ dbt run --profile postgres-test Compiled 1 models, 0 data tests, 0 schema tests, 0 archives, 0 analyses Loading dependency graph file Connecting to postgres. ERROR: Could not connect to the target database. Try `dbt debug` for more information. FATAL: password authentication failed for user "root" (hosted-test3) ✘-1 ~/git/hosted-test [master|✚ 1] 08:32 $ echo $? 1 (hosted-test3) ✔ ~/git/hosted-test [master|✚ 1] 08:32 $ dbt run --profile snowflake-test Compiled 1 models, 0 data tests, 0 schema tests, 0 archives, 0 analyses Loading dependency graph file Connecting to snowflake. ERROR: Could not connect to the target database. Try `dbt debug` for more information. 250001 (08001): failed to connect to DB: kw27752.snowflakecomputing.com:443, proxies=None:None, proxy_user=None, Incorrect username or password was specified. (hosted-test3) ✘-1 ~/git/hosted-test [master|✚ 1] 08:32 $ echo $? 1

drewbanin · 2017-01-12T22:56:36Z

dbt/schema.py

@@ -82,285 +82,3 @@ def string_type(cls, size):

    def __repr__(self):
        return "<Column {} ({})>".format(self.name, self.data_type)
-
-
-class Schema(object):


drewbanin · 2017-01-12T22:57:37Z

dbt/targets.py

@@ -143,7 +143,7 @@ def context(self):

 target_map = {


do we need this file anymore?

This is still used in two places:

(dbt) ✔ ~/git/dbt [adapters/initial|✔] 16:04 $ git grep dbt.targets dbt/schema_tester.py:import dbt.targets dbt/schema_tester.py: return dbt.targets.get_target(target_cfg) dbt/seeder.py:import dbt.targets dbt/seeder.py: self.target = dbt.targets.get_target(run_environment)

These two just don't support Snowflake. I just tested this and I got kind of a strange error:

✘-1 ~/git/hosted-test [master|✚ 1] 16:13 $ dbt seed --profile snowflake-test Encountered an error while reading the project: ERROR Expected project configuration 'host' was not supplied Did you set the correct --profile? Using: snowflake-test Valid profiles: - hosted-test - snowflake-test Encountered an error: Could not run dbt

I'll work on that.

Updated so seed exits with this if run on snowflake:

16:51 $ dbt seed --profile snowflake-test ERROR: `seed` operation is not supported for snowflake.

drewbanin · 2017-01-12T22:58:37Z

requirements.txt

@@ -5,7 +5,7 @@ psycopg2==2.6.1
 sqlparse==0.1.19
 networkx==1.11
 csvkit==0.9.1
-paramiko==2.0.1
-sshtunnel==0.0.8.2
 snowplow-tracker==0.7.2
 celery==3.1.23


celery is a dependency of snowplow-tracker -- do we still need to include it? unsure how it got in the requirements file

drewbanin · 2017-01-12T23:02:00Z

test/integration/base.py

+
+    # horrible hack to support snowflake for right now
+    def transform_sql(self, query):
+        to_return = query


what's going on here haha

rather than maintain two of each seed file, I just replaced BIGSERIAL with BIGINT AUTOINCREMENT hah.... there's definitely a cleaner way & I didn't spend very much time working on this. What do you think?

Oh, I understand. I was thinking about using an ORM for these seed files. That would make it substantially easier to apply existing integration tests to new adapters. SQLAlchemy has dialect support for postgres/redshift/snowflake, for instance. This works for now though

That makes a lot of sense, we should do that.

drewbanin · 2017-01-12T23:03:10Z

test/setup_db.sh

@@ -1,3 +1,4 @@
+set -x


didn't know about this -- that's cool

…adapters/initial

drewbanin · 2017-01-16T19:57:06Z

Great work @cmcarthur -- merge at your convenience

automatic commit by git-black, original commits: 172c7ce

Connor McArthur added 6 commits December 28, 2016 11:42

initial commit, acquire_connection working for postgres with unit test

3145139

cleanup strings, add docker unit test harness

13fe3e5

move execute_without_auto_commit to postgres adapter

973c4ad

wired in w/ strict mode

f4f0b41

turn strict mode on for all tests, not just default ones

d0a6f17

add schema to unit test harness

32e20a3

drewbanin reviewed Jan 6, 2017

View reviewed changes

Connor McArthur added 15 commits January 6, 2017 13:09

merged development

9a46a2e

back to pep8 compliance

c612333

again, improve test harness

7023921

remove now-unused test scripts

e848e59

implement connection sharing w/ test, reset cache on dbt invocation i…

7076a7d

…n case of multiple invocations in one python process

move drop into adapter

b2e31c9

add truncate, rename functions & add master exception handler

0886bc4

back to pep-8 compliance

31502f8

test harness++

f6e0813

snowflake integration test (not really working yet)

4a6750e

added snowflake adapter

5676c34

tearing out most of schema helper

dcb8278

schema helper is dead (long live schema helper)

7a52b80

possibly snowflake-ready?

6251c07

snowflake workin

64ee3eb

cmcarthur mentioned this pull request Jan 11, 2017

Connecting to redshift is a lie #264

Closed

Connor McArthur added 7 commits January 11, 2017 16:44

merged development

d2ea805

snowflake / circle test harness

62f2f68

merged development

6023586

two more tests

ec50446

get back into pep8 compliance

a5e71f0

debug setup_db.sh

9c95a92

display a reasonable status message

3a157fd

add date functions all around

114fb91

drewbanin reviewed Jan 12, 2017

View reviewed changes

Connor McArthur added 12 commits January 13, 2017 16:05

use absolute import to resolve ns conflict

b959fb8

group tests by warehouse type

f5d7be8

Merge branch 'development' of github.com:analyst-collective/dbt into …

2913e81

…adapters/initial

remove targets from seeder, and mark it as not implemented for snowflake

736dcf9

pep8

cd1fe4f

rip snowflake support out for windows

7c0f26b

fix appveyor

6cf9684

only do snowflake imports in snowflake adapter module

887fe85

fix SnowflakeAdapter NameError

2c0e5ec

improved error handling

6be3d44

pep8

6e14eb8

Merge branch 'development' of github.com:analyst-collective/dbt into …

95e4a75

…adapters/initial

update required version of snowflake connector

4970d6d

cmcarthur mentioned this pull request Jan 17, 2017

handle "can't connect" errors more gracefully #253

Closed

Connor McArthur added 7 commits January 20, 2017 13:10

run tests on python 3.6 instead of 3.5

15be495

add python 3.6 to global pyenv

64e4b67

also isntall python 3.6

e30931a

install!

527eaa8

sudo !!

67cc3b4

try python 2.7.12

73cde44

sneak in a logger name change, and CHANGELOG

2b3e8dd

cmcarthur merged commit 172c7ce into development Jan 20, 2017

cmcarthur deleted the adapters/initial branch January 20, 2017 19:00

iknox-fa pushed a commit that referenced this pull request Feb 8, 2022

add snowflake support (#259)

cbed9cc

automatic commit by git-black, original commits: 172c7ce

jtcohen6 mentioned this pull request Sep 12, 2022

Overhaul of Adapter Development Contributing Docs dbt-labs/docs.getdbt.com#1727

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adapters: postgres & redshift #259

adapters: postgres & redshift #259

cmcarthur commented Dec 28, 2016

drewbanin left a comment

drewbanin Jan 6, 2017

drewbanin Jan 6, 2017

drewbanin Jan 6, 2017

drewbanin Jan 6, 2017

drewbanin left a comment

drewbanin Jan 12, 2017

cmcarthur Jan 14, 2017

drewbanin Jan 14, 2017

drewbanin Jan 12, 2017

drewbanin Jan 12, 2017

cmcarthur Jan 14, 2017

drewbanin Jan 12, 2017

drewbanin Jan 12, 2017

cmcarthur Jan 14, 2017

cmcarthur Jan 16, 2017 •

edited

Loading

drewbanin Jan 16, 2017

drewbanin Jan 12, 2017

drewbanin Jan 12, 2017

cmcarthur Jan 14, 2017

cmcarthur Jan 14, 2017

drewbanin Jan 12, 2017

drewbanin Jan 12, 2017

cmcarthur Jan 14, 2017

drewbanin Jan 14, 2017

cmcarthur Jan 16, 2017

drewbanin Jan 12, 2017

drewbanin commented Jan 16, 2017

		from dbt.adapters.postgres import PostgresAdapter


		class RedshiftAdapter(PostgresAdapter):

		schema."""


		@contextmanager

adapters: postgres & redshift #259

adapters: postgres & redshift #259

Conversation

cmcarthur commented Dec 28, 2016

drewbanin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewbanin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmcarthur Jan 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewbanin commented Jan 16, 2017

cmcarthur Jan 16, 2017 •

edited

Loading