-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
snowflake in windows #276
snowflake in windows #276
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Two quick questions for my edification, then merge away
@@ -51,6 +51,7 @@ install: | |||
build: false # Not a C# project, build stuff at the test step instead. | |||
|
|||
before_test: | |||
- "%CMD_IN_ENV% pip install psycopg2==2.6.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this in addition to the requirements.txt
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This installs psycopg2 from a wheel, which links it differently from doing a pip install. I'm not 100% certain tbh but I know it's necessary.
- PYTHON: "C:\\Python27" | ||
PYTHON_VERSION: "2.7.8" | ||
- PYTHON: "C:\\Python35" | ||
PYTHON_VERSION: "3.5.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that Snowflake is only supported on Windows with py35?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implication is actually that dbt 0.7.0 is only supported on Windows with py35/py36. You can't even import the snowflake connector on py27 because of some weird relative imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, noted. Are you able to update the installation docs for windows with this information?
Add integration tests Add accepted_range documentation Fixing invalid yaml in readme Add missing columns: entry Co-authored-by: Claire Carroll <carroll.claire.e@gmail.com> Update changelog
Tests are finally passing