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

Stub out methods at parse time #1413

Merged
merged 4 commits into from
May 1, 2019

Conversation

beckjake
Copy link
Contributor

Stub out many adapter methods at parse time. Unfortunately this doesn't appear to reduce the integration test runtime, but I've verified it does prevent calls to the various adapter methods from running (with a real project) and added unit tests about it. It nicely solves the problem of needing to parse files before being able to populate the cache by stubbing out everything that might want the cache at parse-time.

already_exists returns False for consistency with get_relation.

I'm not sure this is the most perfect way of doing it, but using it does feel reasonably pleasant and it seems more sustainable than overriding it manually inside the parser or something. I stole the available.parse = available_parse idea from mock's patch.object.

I think the biggest downside is that you have to decide whether or not to stub it out in parsing when you mark them available, not at actual implementation time, because the wrapper only delegates the top-level call, and subsequent accesses to self hit the wrapped adapter only. I think that's pretty much fine, but I did want to call it out.

Includes some unit tests
Update integration tests to handle the fact that sometimes we now fail at runtime
@beckjake beckjake force-pushed the feature/stub-adapter-in-parsing branch from 25189ed to 0f1c154 Compare April 24, 2019 20:37
@beckjake beckjake requested a review from drewbanin April 30, 2019 16:15
Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

This LGTM! There are some pretty strong assumptions that have motivated this change.... let's be sure to test the heck out of this on a variety of representative dbt projects.

@beckjake beckjake merged commit 3c8bbdd into dev/wilt-chamberlain May 1, 2019
@beckjake beckjake deleted the feature/stub-adapter-in-parsing branch May 1, 2019 16:39
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