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

broken commit: update internals of insert by period #319

Merged
merged 4 commits into from
Jan 6, 2021

Conversation

clrcrl
Copy link
Contributor

@clrcrl clrcrl commented Jan 6, 2021

@jtcohen6 — this is what is broken, but I can't quite get it to work! not sure how to access the internals of the AdapterResponse class. Figured you would have a better idea than me :)

Also — this would be classed as a breaking change, so we might want to write some logic about handling this on different versions of dbt to avoid the break.

I was curious as to how this snuck in, and went back to #309 — turns out it silently failed back then! I think because the "test" step passed, even though the "run" step failed. Now that we are using -x, the job exits, causing other tests to fail, and the resultant error message.

I think the problem here is that we run the steps via the run_test.sh file, when potentially we want them as separate (parameterized?) steps in the CircleCI workflow.

@clrcrl clrcrl requested a review from jtcohen6 January 6, 2021 17:20
@jtcohen6
Copy link
Contributor

jtcohen6 commented Jan 6, 2021

Nice work on this one! Check out #320. I think we can still manage this as a patch release.

Now that we are using -x, the job exits, causing other tests to fail, and the resultant error message.

Which is a good thing!

I think the problem here is that we run the steps via the run_test.sh file, when potentially we want them as separate (parameterized?) steps in the CircleCI workflow.

Ideally the bash script would stop on the first error. We could maybe get this for free by adding set -e to the top of the script? There are known caveats, but this worked for me locally—I intentionally broke a model, it failed fast, the script stopped.

@jtcohen6 jtcohen6 mentioned this pull request Jan 6, 2021
9 tasks
@clrcrl clrcrl marked this pull request as ready for review January 6, 2021 19:03
@clrcrl clrcrl merged commit 6d7ff51 into update/circleci-workflows Jan 6, 2021
@joellabes joellabes deleted the 0-19-0-updates branch October 21, 2021 22:33
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.

2 participants