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 retry docs generate by marking certain args as False by default. #10742

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240919-180837.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Fix NoSuchOption errors when running `dbt retry` after `dbt docs generate`.
time: 2024-09-19T18:08:37.450919604+02:00
custom:
Author: jordivandooren
Issue: "10741"
1 change: 0 additions & 1 deletion core/dbt/task/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ def __init__(self, args: Flags, config: RuntimeConfig) -> None:
cli_command = CMD_DICT.get(self.previous_command_name) # type: ignore
# Remove these args when their default values are present, otherwise they'll raise an exception
args_to_remove = {
"show": lambda x: True,
"resource_types": lambda x: x == [],
"warn_error_options": lambda x: x == {"exclude": [], "include": []},
}
Expand Down
5 changes: 4 additions & 1 deletion core/dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def __contains__(self, name) -> bool:
# cli args and flags, which is more complete than just the cli args.
# If new args are added that are false by default (particularly in the
# global options) they should be added to the 'default_false_keys' list.
def args_to_dict(args):
def args_to_dict(args) -> dict:
var_args = vars(args).copy()
# update the args with the flags, which could also come from environment
# variables or project_flags
Expand All @@ -355,6 +355,9 @@ def args_to_dict(args):
"log_cache_events",
"store_failures",
"use_experimental_parser",
"static",
"empty_catalog",
"show",
)
default_empty_yaml_dict_keys = ("vars", "warn_error_options")
if key in default_false_keys and var_args[key] is False:
Expand Down
55 changes: 55 additions & 0 deletions tests/unit/test_flag_translation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import click
import pytest

from dbt.cli import main as cli_main
from dbt.cli.types import Command
from dbt.task.retry import CMD_DICT
from dbt.utils import args_to_dict


def is_problematic_option(option: click.Option) -> bool:
return (
option.is_flag and not option.default and not option.secondary_opts and option.expose_value
)


def get_problemetic_options_for_command(command: click.Command) -> list[str]:
"""
Get boolean flags of a ClickCommand that are False by default, do not
have a secondary option (--no-*), and expose their value.
Why do we care? If not dealt with, these arguments are stored in run_results.json
and converted to non-existent --no-* options when running dbt retry.
"""
return [
option.name
for option in command.params
if isinstance(option, click.Option) and is_problematic_option(option)
]


def get_commands_supported_by_retry() -> list[click.Command]:
command_names = [convert_enum_to_command_function_name(value) for value in CMD_DICT.values()]
return [getattr(cli_main, name) for name in command_names]


def convert_enum_to_command_function_name(enum: Command) -> str:
return "_".join(enum.to_list()).replace("-", "_")


class FlagsDummy:
def __init__(self, args: dict[str, bool]):
self.__dict__ = args


@pytest.mark.parametrize("command", get_commands_supported_by_retry())
def test_flags_problematic_for_retry_are_dealt_with(command: click.Command):
"""
For each command supported by retry, get a list of flags that should
not be converted to --no-* when False, and assert if args_to_dict correctly
skips it.
"""
flag_names = get_problemetic_options_for_command(command)
flags = FlagsDummy({name: False for name in flag_names})
args_dict = args_to_dict(flags)
for flag_name in flag_names:
assert flag_name not in args_dict, f"add {flag_name} to default_false_keys in args_to_dict"