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

Undefined name: import shutil for line 60 #1572

Merged
merged 1 commit into from
Jun 26, 2019

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Jun 26, 2019

No description provided.

@drewbanin
Copy link
Contributor

Hey @cclauss - thanks for opening this PR! I believe we use shutil in that file to remove temporary directories that are created in the integration tests.

Can you tell me more about why you've submitted this PR?

@cclauss
Copy link
Contributor Author

cclauss commented Jun 26, 2019

That code currently throws a NameError because shutil is an undefined name in this context. Since that code is wrapped in a bare exception the NameError is silently swallowed which makes this block of code is a no-op.

@drewbanin
Copy link
Contributor

ha, yes! Very sorry - i thought you were removing this line, not adding it..... that should teach me to review PRs before I've finished my coffee 🤦‍♂

Sure, this LGTM, let's merge it :)

@drewbanin drewbanin merged commit 7d1fed2 into dbt-labs:dev/wilt-chamberlain Jun 26, 2019
@cclauss cclauss deleted the patch-1 branch June 26, 2019 14:40
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