make tracking use the profile directory, and suppress errors (#1180) #1186
Conversation
beckjake
force-pushed
the
fix/tracking
branch
from
e09388e
to
50e06b4
Compare
beckjake
force-pushed
the
fix/tracking
branch
from
50e06b4
to
808ed75
Compare
| from dbt.config import Project, RuntimeConfig, DbtProjectError, \ | ||
| DbtProfileError, PROFILES_DIR, read_config, \ | ||
| send_anonymous_usage_stats, colorize_output, read_profiles | ||
| from dbt.config import Project, UserConfig, RuntimeConfig, PROFILES_DIR, \ |
There was a problem hiding this comment.
i think we lost DbtProjectError here
There was a problem hiding this comment.
It was a double-import that I incorrectly added at some point - see 2 lines down where we import it again from dbt.exceptions! Same goes for DbtProfileError.
There was a problem hiding this comment.
dang. idk where it is in the code, but i saw this when supplying a --profiles-dir that did not contain a profiles.yml:
$ dbt run --profiles-dir .
Encountered an error:
name 'DbtProjectError' is not defined
Traceback (most recent call last):
File "/Users/drew/fishtown/dbt/dbt/main.py", line 247, in invoke_dbt
cfg = RuntimeConfig.from_args(parsed)
File "/Users/drew/fishtown/dbt/dbt/config.py", line 961, in from_args
cli_vars=cli_vars
File "/Users/drew/fishtown/dbt/dbt/config.py", line 766, in from_args
threads_override=threads_override
File "/Users/drew/fishtown/dbt/dbt/config.py", line 716, in from_raw_profiles
"Could not find profile named '{}'".format(profile_name)
dbt.exceptions.DbtProjectError: Runtime Error
Could not find profile named 'debug'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/Users/drew/fishtown/dbt/dbt/main.py", line 77, in main
results, succeeded = handle_and_check(args)
File "/Users/drew/fishtown/dbt/dbt/main.py", line 151, in handle_and_check
task, res = run_from_args(parsed)
File "/Users/drew/fishtown/dbt/dbt/main.py", line 191, in run_from_args
res = invoke_dbt(parsed)
File "/Users/drew/fishtown/dbt/dbt/main.py", line 248, in invoke_dbt
except DbtProjectError as e:
NameError: name 'DbtProjectError' is not defined
There was a problem hiding this comment.
(we actually import DbtProjectError twice below)
There was a problem hiding this comment.
We actually import DbtProfileError twice in a row, ugh... fixed
beckjake
force-pushed
the
fix/tracking
branch
from
cad67ea
to
5deb7e8
Compare
| yaml.dump(user, fh) | ||
|
|
||
| return user | ||
|
|
||
| def get_cookie(self): | ||
| if not os.path.isfile(COOKIE_PATH): | ||
| if not os.path.isfile(self.cookie_path): |
There was a problem hiding this comment.
is it possible to defer the creation of this cookie until the profile is loaded? With the current implementation, dbt will create a cookie if it doesn't exist in the specified profile directory. That means if you errantly specify an incorrect profile dir, dbt will litter .user.yml files all over the place. This won't affect tracking and i don't think it will cause any problems for the end-user, but it's not so polite :)
I figure we can only create the cookie if dbt is able to find and load the profiles.yml file. To be sure: I tested this out, and dbt does not send events if the profiles.yml file is unloadable, which is good. This is mostly a cosmetic/tidyness comment
There was a problem hiding this comment.
It's possible, but not easy - I would estimate it at a few days of work, as a ton of main.py would have to be refactored. We start doing tracking things before we load a profile - really before we ever even start inspecting which command was passed, so we don't even know if we will load a profile! And sometimes, in order to load a profile we actually need to load the project first so we can figure out the profile name, etc... Accordingly, per our slack conversation, I'm going to merge this as-is - we should get this fix in to grace-kelly, even if it's a bit less than optimal.
I am very enthusiastic about the idea of refactoring main.py to support this behavior - currently even answering the question "does this command require profile, project, both or neither" requires checking multiple functions and carefully walking through behavior, and I don't like that. We could also handle the issues raised in #1189 when/if we do that.
There was a problem hiding this comment.
totally agree @beckjake, thanks for the color. Ship it!
Fixes #1180
I wanted to make the tracking code use the profile we were going to end up loading later, but our initialization is pretty complicated with various subcommands needing profile/project/both/neither getting those initialized at various times, to the point where it would have been a very large change.
We do need to consider someday about how we want to handle this global state around tracking. I think it should be tied into the Profile object somehow, but that's a lot of refactoring to do, and I want this to get into grace-kelly.