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

Switch to CircleCI workflow #316

Merged
merged 6 commits into from
Jan 6, 2021
Merged

Switch to CircleCI workflow #316

merged 6 commits into from
Jan 6, 2021

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Jan 4, 2021

resolves #315

  • Use 5 threads (Postgres, Redshift) or 10 (Snowflake, BigQuery)—an arbitrary number!
  • Allows us to run tests on different warehouses (Redshift, Snowflake, BigQuery) simultaneously rather than in sequence
    • Removes the caching of dependencies. (a) I think it's pretty fast to install dbt. (b) Helpful if we need to test with a new (prerelease) version of dbt, without needing to manually invalidate cache. (c) I'm not entirely sure how to do these with workflows, and I know it can be tricky to share cache across concurrent jobs—there are plenty of docs to read

This is a:

  • bug fix PR with no breaking changes — please ensure the base branch is master
  • new functionality — please ensure the base branch is the latest dev/ branch
  • a breaking change — please ensure the base branch is the latest dev/ branch

@jtcohen6 jtcohen6 requested a review from clrcrl January 4, 2021 15:01
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Jan 4, 2021

Screen Shot 2021-01-04 at 4 07 38 PM

Copy link
Contributor

@clrcrl clrcrl left a comment

Choose a reason for hiding this comment

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

This looks really great! Curious why we decided to have the three clouds depend on postgres, I'm guessing to catch errors early?

@clrcrl
Copy link
Contributor

clrcrl commented Jan 4, 2021

While we're here, I wouldn't hate adding the -x flag to the commands!

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Jan 4, 2021

I'm guessing to catch errors early?

Yeah, that was my initial thought, but we don't need to do it that way—the tests are so fast, it's not like it saves us very much.

While we're here, I wouldn't hate adding the -x flag to the commands!

Good call!

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Jan 4, 2021

This is unexpected:

Compilation Error in macro materialization_insert_by_period_default (macros/materializations/insert_by_period_materialization.sql)
  'dbt.utils.AttrDict object' has no attribute 'status'
Encountered an error:
FailFast Error in model test_insert_by_period (models/materializations/test_insert_by_period.sql)
  Failing early due to test failure or runtime error

I wonder if this is the result of a weird interaction with --fail-fast?

@clrcrl
Copy link
Contributor

clrcrl commented Jan 4, 2021

(I kind of liked the "postgres first" DAG)

@clrcrl
Copy link
Contributor

clrcrl commented Jan 4, 2021

oh of course insert by period fails. UGH! let me check it out

edit: I actually think this might because I kicked off tests on a different branch, messing with the redshift env that had already been built, which is exactly what #317 tries to solve!

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Jan 6, 2021

Hm I don't think this is a matter of the concurrent run (though that is a potential problem!). I ran drop schema dbt_utils_integration_tests_redshift cascade, reran from failed, and still see the compilation error in materialization_insert_by_period_default.

What's weird is that I was able to dbt run -x --target redshift locally with success, including test_insert_by_period.

@jtcohen6 jtcohen6 merged commit 3d47bdc into master Jan 6, 2021
@jtcohen6 jtcohen6 deleted the update/circleci-workflows branch January 6, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase threads for Integration Test suite
2 participants