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 test_builtin_invocation_args_dict_function #6898

Merged
Show file tree
Hide file tree
Changes from 6 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Tracking this in #6903

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it turns out renaming a symbol in some places and not others is confusing AF. Might as well do it all at once since it's going to require changes in a medium sized pile of tests that have a manually set value for send stats (not sure why, most of them have nothing to do with tracking).
/shrug

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
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
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(
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Can we rewrite this as a loop instead?

(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)
iknox-fa marked this conversation as resolved.
Show resolved Hide resolved

# The result should include a dictionary of all flags with values that aren't None
expected = (
"'send_anonymous_usage_stats': False",
iknox-fa marked this conversation as resolved.
Show resolved Hide resolved
"'quiet': False",
"'no_print': False",
iknox-fa marked this conversation as resolved.
Show resolved Hide resolved
"'print': True",
"'cache_selected_only': False",
"'macro': 'validate_invocation'",
"'args': '{my_variable: test_variable}'",
iknox-fa marked this conversation as resolved.
Show resolved Hide resolved
"'args': {'my_variable': 'test_variable'}",
"'which': 'run-operation'",
"'rpc_method': 'run-operation'",
iknox-fa marked this conversation as resolved.
Show resolved Hide resolved
"'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
53 changes: 41 additions & 12 deletions tests/unit/test_cli_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,54 @@ 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",
Copy link
Contributor

Choose a reason for hiding this comment

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

@MichelleArk
Hilariously the tests were still passing because they were just testing that DO_NOT_TRACK was honored but since boolean logic is easy to get wrong, especially when renaming things I expanded the test matrix a bit. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!! Looks great.

[
("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,
):
# patch the env
monkeypatch.setenv("DO_NOT_TRACK", do_not_track)

# set the param
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 is expected_anonymous_usage_stats

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