Fix project dir argument when running debug (#1733)

[franloza]

Fixes #1733

I did not make the class DebugTask inherited from RequiresProjectTask because it raised a dbt.exceptions.RuntimeException in get_nearest_project_dir() function if dbt_project.yml was not found. Instead, I just used the argument project_dir from arguments as first choice and set current directory as fall-back value.

The included test checks that, even if a dbt_project.yml is in the current directory, the --project-dir argument is used, leading to a "not found" error.

[cla-bot]

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @franloza

[drewbanin]

Thanks for making this PR @franloza!

I gave this a quick spin locally and I noticed that the code change here doesn't have the effected specified in the PR description. Specifically, I saw that when running dbt debug --project-dir path/to/project, dbt still looked for the dbt_project.yml file in the cwd.

I think the relevant method codepath might be around here: https://github.com/fishtown-analytics/dbt/blob/cc491904bc7ed5c27a395e3a1936e57e4221354a/core/dbt/config/project.py#L391-L393

This method is called via _load_project in the DebugTask: https://github.com/fishtown-analytics/dbt/blob/cc491904bc7ed5c27a395e3a1936e57e4221354a/core/dbt/task/debug.py#L122-L133

I think we'll want to update the _load_project method to construct a Project object using args.project_dir. Let me know if you have any questions or further thoughts here. I'll kick off the tests regardless so we can see how they fare :)

(also: @cla-bot check)

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[franloza]

Yes, you were completely right @drewbanin. As you mentioned, the added test only checked that we were looking outside current directory and returned the error in the first if statement:

if not os.path.exists(self.project_path):
    self.project_fail_details = FILE_NOT_FOUND
    return red('ERROR not found')

However, we were not covering the case of specifing an existent project.yml file outside current directory. I have added a test to cover this case and updated _load_project method to use Project.from_project_root instead of Project.from_current_directory. This way, we are using always --project-dir as first choice and current directory as fall-back value.

[drewbanin]

Thanks for the update @franloza! I kicked off the tests here and I'm adding @beckjake to review. This LGTM when the tests pass :)

[drewbanin]

Looks like this line is too long per our styleguide:

core/dbt/task/debug.py:129:80: E501 line too long (85 > 79 characters)

via: https://circleci.com/gh/fishtown-analytics/dbt/19769?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Can you split this onto two lines?

[beckjake]

Looks good to me. Two tiny comments + the flake8 fix that drew noted and this will be great. Thanks for writing great tests!

[beckjake]

You shouldn't need to call use_default_project unless you do things with the project/profile as part of the test - setUp will call it (some old tests still have it, so I understand where this came from!)

[franloza]

Yes, you're right @beckjake . I did not realize that it is called in setUp method. I will take it into account for the next time, thanks!

[beckjake]

Can you please add a newline at the end of this file? looking at stuff in the terminal can get annoying without trailing newlines.

[franloza]

I have fixed style code style violations and removed self.use_default_project() from one test. Really happy to contribute, I think you all are doing a great job with tool 👍

[beckjake]

Looks great, I've kicked off the tests for a final run. Thanks for your contribution @franloza!