Add logging for a file path that can't be cleaned

[emilieschario]

Closes #2059

In a case where your dbt_project.yml file has:

source-paths: ["models"]

clean-targets:
  - "models"
  - "dbt_modules"

when you run dbt clean you'll see a message that looks like:

Checking models/*
Checking dbt_modules/*
 Cleaned dbt_modules/*

Because it doesn't tell you you did something wrong, you may not notice that the model/* file wasn't cleaned (rightfully so!). This MR proposes that the messaging be:

Checking model/*
model/* cannot be cleaned.
Checking dbt_modules/*
 Cleaned dbt_modules/*

No functional differences, just more details.

[emilieschario]

@drewbanin I think these tests are failing because someone internal has to kick them off

[drewbanin]

Thanks for making this PR @emilieschario! I just kicked off the tests here.

This is super minor, but what's your rationale behind not indenting the log line that you added here? And what would you think about updating this line to be even more apparent with something like:

 ERROR: not cleaning {}/* because it is protected

Let me know what you think!

[emilieschario]

On the not indenting piece, it was being it was hard to anticipate the length, so it could be a short name like model or it could be kjdhfskjfdlakjldkj so

Checking model/*
kjdhfskjfdlakjldkj/* cannot be cleaned.
Checking dbt_modules/*
 Cleaned dbt_modules/*

But I love love love the error message! I'm going to update now so that you can kick off tests again, if you're still around

[emilieschario]

Updated @drewbanin!

[drewbanin]

Thanks @emilieschario! This LGTM, but I have terrible news: your new change has a line that's longer than 80 characters, so it won't pass our code formatting CI check.

Can you try changing this line:

                logger.info("ERROR: not cleaning {}/* because it is protected".format(path))

to look like:

                logger.info("ERROR: not cleaning {}/* because it is "
                            "protected".format(path))

This will fix the pep8 error that might show up in Circle momentarily :)

[emilieschario]

Thanks for your patience here @drewbanin! Updated!

[drewbanin]

np! Thanks for that - just kicked off the tests again and will get this merge when they pass :)

[drewbanin]

hey @emilieschario - this code is definitely read to roll, but in testing this feature, I did notice a big problem!

Check out this code (unrelated to your PR): https:/fishtown-analytics/dbt/pull/2060/files#diff-0927dc49a0a3b3ad824b956c8f0abb3fR21-R26

On line 24 here, we call:

protected_abs_paths = [os.path.abspath for p in protected_paths]

The problem is that we're not calling the abspath function, so protected_abs_paths takes on a value like:

> protected_abs_paths
[<function abspath at 0x101f3e950>, <function abspath at 0x101f3e950>, <function abspath at 0x101f3e950>]

These list elements are pointers to the abspath method, not absolute paths themselves! As a result, the __is_protected_path method does not actually check if paths are protected :/

If you're interested in fixing that issue in the scope of this PR, I think that would be a really good and helpful change. If not, we can totally queue up a second ticket to address it.

Let me know!

[emilieschario]

woah. so why didn't my file get cleaned before if it wasn't because it was being deemed a file path? how did I come to this conclusion? 😕

I think I got it. Thanks for giving me the chance to do that.

[drewbanin]

Thanks @emilieschario - looks great! I'm kicking off the tests one last time, then will merge this when they pass :)

[emilieschario]

Thanks for the opportunity @drewbanin!