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

Fix Project Env Var Tests #6916

Merged
merged 8 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,9 @@ def assign_params(ctx, params_assigned_from_default):
object.__setattr__(self, "LOG_PATH", log_path)

# Support console DO NOT TRACK initiave
object.__setattr__(
self,
"ANONYMOUS_USAGE_STATS",
False
if os.getenv("DO_NOT_TRACK", "").lower() in ("1", "t", "true", "y", "yes")
else True,
)
if os.getenv("DO_NOT_TRACK", "").lower() in ("1", "t", "true", "y", "yes"):
object.__setattr__(self, "SEND_ANONYMOUS_USAGE_STATS", False)

# Check mutual exclusivity once all flags are set
self._assert_mutually_exclusive(
params_assigned_from_default, ["WARN_ERROR", "WARN_ERROR_OPTIONS"]
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def invoke(self, args: List[str]) -> Tuple[Optional[List], bool]:
epilog="Specify one of these sub-commands and you can find more help from there.",
)
@click.pass_context
@p.anonymous_usage_stats
@p.send_anonymous_usage_stats
@p.cache_selected_only
@p.debug
@p.enable_legacy_logger
Expand Down
11 changes: 4 additions & 7 deletions core/dbt/cli/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@
from dbt.cli.resolvers import default_project_dir, default_profiles_dir
from dbt.version import get_version_information

# TODO: The name (reflected in flags) is a correction!
# The original name was `SEND_ANONYMOUS_USAGE_STATS` and used an env var called "DBT_SEND_ANONYMOUS_USAGE_STATS"
# Both of which break existing naming conventions (doesn't match param flag).
# This will need to be fixed before use in the main codebase and communicated as a change to the community!
anonymous_usage_stats = click.option(
"--anonymous-usage-stats/--no-anonymous-usage-stats",
envvar="DBT_ANONYMOUS_USAGE_STATS",
# TODO: Rename this to meet naming conventions (the word "send" is redundant)
send_anonymous_usage_stats = click.option(
"--send-anonymous-usage-stats/--no-send-anonymous-usage-stats",
envvar="DBT_SEND_ANONYMOUS_USAGE_STATS",
help="Send anonymous usage stats to dbt Labs.",
default=True,
)
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/cli/requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def wrapper(*args, **kwargs):
set_flags(flags)

# Tracking
initialize_from_flags(flags.ANONYMOUS_USAGE_STATS, flags.PROFILES_DIR)
initialize_from_flags(flags.SEND_ANONYMOUS_USAGE_STATS, flags.PROFILES_DIR)
ctx.with_resource(track_run(run_command=flags.WHICH))

# Logging
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def __post_init__(self):
self.user_id = tracking.active_user.id

if self.send_anonymous_usage_stats is None:
self.send_anonymous_usage_stats = get_flags().ANONYMOUS_USAGE_STATS
self.send_anonymous_usage_stats = get_flags().SEND_ANONYMOUS_USAGE_STATS

@classmethod
def default(cls):
Expand Down
1 change: 0 additions & 1 deletion core/dbt/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ def get_flag_dict():
"version_check",
"fail_fast",
"send_anonymous_usage_stats",
"anonymous_usage_stats",
"printer_width",
"indirect_selection",
"log_cache_events",
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def handle_and_check(args):
# Set flags from args, user config, and env vars
user_config = read_user_config(flags.PROFILES_DIR) # This is read again later
flags.set_from_args(parsed, user_config)
dbt.tracking.initialize_from_flags(flags.ANONYMOUS_USAGE_STATS, flags.PROFILES_DIR)
dbt.tracking.initialize_from_flags(flags.SEND_ANONYMOUS_USAGE_STATS, flags.PROFILES_DIR)
# Set log_format from flags
parsed.cls.set_log_format()

Expand Down
3 changes: 3 additions & 0 deletions core/dbt/tests/fixtures/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ def __init__(
test_data_dir,
test_schema,
database,
logs_dir,
test_config,
):
self.project_root = project_root
Expand All @@ -391,6 +392,7 @@ def __init__(
self.test_data_dir = test_data_dir
self.test_schema = test_schema
self.database = database
self.logs_dir = logs_dir
self.test_config = test_config
self.created_schemas = []

Expand Down Expand Up @@ -486,6 +488,7 @@ def project(
test_data_dir=test_data_dir,
test_schema=unique_schema,
database=adapter.config.credentials.database,
logs_dir=logs_dir,
test_config=test_config,
)
project.drop_test_schema()
Expand Down
4 changes: 2 additions & 2 deletions core/dbt/tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,10 @@ def process(self, record):
)


def initialize_from_flags(anonymous_usage_stats, profiles_dir):
def initialize_from_flags(send_anonymous_usage_stats, profiles_dir):
# Setting these used to be in UserConfig, but had to be moved here
global active_user
if anonymous_usage_stats:
if send_anonymous_usage_stats:
active_user = User(profiles_dir)
try:
active_user.initialize()
Expand Down
4 changes: 2 additions & 2 deletions test/unit/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ def test__build_flat_graph(self):
def test_metadata(self, mock_user):
mock_user.id = 'cfc9500f-dc7f-4c83-9ea7-2c581c1b38cf'
dbt.events.functions.EVENT_MANAGER.invocation_id = '01234567-0123-0123-0123-0123456789ab'
set_from_args(Namespace(ANONYMOUS_USAGE_STATS=False), None)
set_from_args(Namespace(SEND_ANONYMOUS_USAGE_STATS=False), None)
now = datetime.utcnow()
self.assertEqual(
ManifestMetadata(
Expand All @@ -450,7 +450,7 @@ def test_metadata(self, mock_user):
def test_no_nodes_with_metadata(self, mock_user):
mock_user.id = 'cfc9500f-dc7f-4c83-9ea7-2c581c1b38cf'
dbt.events.functions.EVENT_MANAGER.invocation_id = '01234567-0123-0123-0123-0123456789ab'
set_from_args(Namespace(ANONYMOUS_USAGE_STATS=False), None)
set_from_args(Namespace(SEND_ANONYMOUS_USAGE_STATS=False), None)
metadata = ManifestMetadata(
project_id='098f6bcd4621d373cade4e832627b4f6',
adapter_type='postgres',
Expand Down
7 changes: 4 additions & 3 deletions tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import os
import re
import yaml

from dbt.cli.main import dbtUsageException
from dbt.tests.util import run_dbt

MODELS__MODEL_SQL = """
Expand Down Expand Up @@ -86,9 +88,8 @@ def test_badproject(self, project):
self.check_project(splitout)

def test_not_found_project(self, project):
run_dbt(["debug", "--project-dir", "nopass"], expect_pass=False)
splitout = self.capsys.readouterr().out.split("\n")
self.check_project(splitout, msg="ERROR not found")
with pytest.raises(dbtUsageException):
run_dbt(["debug", "--project-dir", "nopass"])

def test_invalid_project_outside_current_dir(self, project):
# create a dbt_project.yml
Expand Down
28 changes: 15 additions & 13 deletions tests/functional/context_methods/test_builtin_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@

macros__validate_invocation_sql = """
{% macro validate_invocation(my_variable) %}
-- check a specific value
{{ log("use_colors: "~ invocation_args_dict['use_colors']) }}
-- whole dictionary (as string)
{{ log("invocation_result: "~ invocation_args_dict) }}
{% endmacro %}
"""
Expand Down Expand Up @@ -61,7 +64,11 @@ def parse_json_logs(json_log_output):

def find_result_in_parsed_logs(parsed_logs, result_name):
return next(
(item for item in parsed_logs if result_name in item["info"].get("msg", "msg")),
(
item["data"]["msg"]
for item in parsed_logs
if result_name in item["data"].get("msg", "msg")
),
False,
)

Expand Down Expand Up @@ -104,27 +111,22 @@ def test_builtin_invocation_args_dict_function(self, project):
)

parsed_logs = parse_json_logs(log_output)
result = find_result_in_parsed_logs(parsed_logs, "invocation_result")

use_colors = result = find_result_in_parsed_logs(parsed_logs, "use_colors")
assert use_colors == "use_colors: True"
invocation_dict = find_result_in_parsed_logs(parsed_logs, "invocation_result")
assert result

# Result is checked in two parts because profiles_dir is unique each test run
expected = "invocation_result: {'debug': True, 'log_format': 'json', 'write_json': True, 'use_colors': True, 'printer_width': 80, 'version_check': True, 'partial_parse': True, 'static_parser': True, 'profiles_dir': "
assert expected in str(result)

# The result should include a dictionary of all flags with values that aren't None
expected = (
"'send_anonymous_usage_stats': False",
"'quiet': False",
"'no_print': False",
"'print': True",
"'cache_selected_only': False",
"'macro': 'validate_invocation'",
"'args': '{my_variable: test_variable}'",
"'args': {'my_variable': 'test_variable'}",
"'which': 'run-operation'",
"'rpc_method': 'run-operation'",
"'indirect_selection': 'eager'",
)
for element in expected:
assert element in str(result)
assert all(element in invocation_dict for element in expected)

def test_builtin_dbt_metadata_envs_function(self, project, monkeypatch):
envs = {
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/partial_parsing/test_pp_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def test_profile_env_vars(self, project):
with pytest.raises(FailedToConnectError):
run_dbt(["run"], expect_pass=False)

log_output = Path(project.project_root, "logs", "dbt.log").read_text()
log_output = Path(project.logs_dir, "dbt.log").read_text()
Copy link
Contributor

@MichelleArk MichelleArk Feb 9, 2023

Choose a reason for hiding this comment

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

I ended up doing something very similar in my attempt to fix this but @gshank raises a really good point in this comment - why is this test is now throwing an error in click-cli when it doesn't in main?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea, but not sure if it's worth investigating since this is the intended behavior.

assert "env vars used in profiles.yml have changed" in log_output

manifest = get_manifest(project.project_root)
Expand Down
51 changes: 39 additions & 12 deletions tests/unit/test_cli_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,52 @@ def test_log_path_default(self, run_context):
assert getattr(flags, "LOG_PATH") == "logs"

@pytest.mark.parametrize(
"do_not_track,expected_anonymous_usage_stats",
"set_stats_param,do_not_track,expected_anonymous_usage_stats",
[
("1", False),
("t", False),
("true", False),
("y", False),
("yes", False),
("false", True),
("anything", True),
("2", True),
# set_stats_param = default, DNT = True, expected = False
("default", "1", False),
("default", "t", False),
("default", "true", False),
("default", "y", False),
("default", "yes", False),
# set_stats_param = default, DNT = false, expected = True
("default", "false", True),
("default", "anything", True),
# set_stats_param = True, DNT = True, expected = False
(True, "1", False),
(True, "t", False),
(True, "true", False),
(True, "y", False),
(True, "yes", False),
# set_stats_param = True, DNT = false, expected = True
(True, "false", True),
(True, "anything", True),
(True, "2", True),
# set_stats_param = False, DNT = True, expected = False
(False, "1", False),
(False, "t", False),
(False, "true", False),
(False, "y", False),
(False, "yes", False),
# set_stats_param = False, DNT = False, expected = False
(False, "false", False),
(False, "anything", False),
(False, "2", False),
],
)
def test_anonymous_usage_state(
self, monkeypatch, run_context, do_not_track, expected_anonymous_usage_stats
self,
monkeypatch,
run_context,
set_stats_param,
do_not_track,
expected_anonymous_usage_stats,
):
monkeypatch.setenv("DO_NOT_TRACK", do_not_track)

if set_stats_param != "default":
run_context.params["send_anonymous_usage_stats"] = set_stats_param
flags = Flags(run_context)
assert flags.ANONYMOUS_USAGE_STATS == expected_anonymous_usage_stats
assert flags.SEND_ANONYMOUS_USAGE_STATS == expected_anonymous_usage_stats

def test_empty_user_config_uses_default(self, run_context, user_config):
flags = Flags(run_context, user_config)
Expand Down