[CT-2594] [Regression] WarnErrorOptions in invocation_args_dict is not JSON serialisable

[jared-rimmer]

Is this a new bug in dbt-core?

• I believe this is a new bug in dbt-core
• I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

• In dbt 1.5.0 the value for the key warn_error_options in the invocation_arg_dict object is a Python object which looks like "warn_error_options": WarnErrorOptions(include=[], exclude=[]),

Expected Behavior

• In dbt 1.4.x and earlier the value for the key warn_error_options in the invocation_arg_dict object was a string of a dict object which looks like "warn_error_options\": {\"exclude\":[],\"include\":[]}

Steps To Reproduce

1. Invoke dbt in 1.4.x with the warn options flag
2. Do the same in 1.5.0
3. Observe the difference invocation_arg_dict object

Relevant log output

No response

Environment

- OS: Mac
- Python: 3.8
- dbt: 1.5.0

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

All of them

[jtcohen6]

Thanks @jared-rimmer! This is similar to:

• [CT-2516] [Bug] Upgrade to 1.5.0 generates "TypeError: argument of type 'PosixPath' is not iterable" on model runs. #7495.

I agree that we should serialize WarnErrorOptions somehow—whether to a real Python dictionary, or a JSON string—before exposing it to the Jinja context. The right spot to do that is probably in args_to_dict:

dbt-core/core/dbt/utils.py

Lines 614 to 656 in 45d6145

	# This is used to serialize the args in the run_results and in the logs.
	# We do this separately because there are a few fields that don't serialize,
	# i.e. PosixPath, WindowsPath, and types. It also includes args from both
	# 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):
	var_args = vars(args).copy()
	# update the args with the flags, which could also come from environment
	# variables or user_config
	flag_dict = flags.get_flag_dict()
	var_args.update(flag_dict)
	dict_args = {}
	# remove args keys that clutter up the dictionary
	for key in var_args:
	if key.lower() in var_args and key == key.upper():
	# skip all capped keys being introduced by Flags in dbt.cli.flags
	continue
	if key in ["cls", "mp_context"]:
	continue
	if var_args[key] is None:
	continue
	# TODO: add more default_false_keys
	default_false_keys = (
	"debug",
	"full_refresh",
	"fail_fast",
	"warn_error",
	"single_threaded",
	"log_cache_events",
	"store_failures",
	"use_experimental_parser",
	)
	default_empty_yaml_dict_keys = ("vars", "warn_error_options")
	if key in default_false_keys and var_args[key] is False:
	continue
	if key in default_empty_yaml_dict_keys and var_args[key] == "{}":
	continue
	# this was required for a test case
	if isinstance(var_args[key], PosixPath) or isinstance(var_args[key], WindowsPath):
	var_args[key] = str(var_args[key])
	dict_args[key] = var_args[key]
	return dict_args

Is that something you'd be interested in poking around with? Otherwise, we'll try to include this for a v1.5 patch, but I view it as lower priority relative to other bugs/regressions.

[emmyoop]

May be able to just pass the dbtclass mixin to the warnerroroptions.

[QMalcolm]

Interesting the WarnErrorOptions class already uses the dbtClassMixin 🤔 Specifically WarnErrorOptions inherits IncludeExclude and IncludeExclude inherits dbtClassMixin. So having dbtClassMixin as part of WarnErrorOptions doesn't resolve the issue. It appears to be more of an issue of dictifying nested objects.

[QMalcolm]

@jtcohen6 you said

I agree that we should serialize WarnErrorOptions somehow
Does that mean we're not specifically aiming to give the exact previous output, but just that it is serialized in a more friendly way. I.e. it doesn't need to be "warn_error_options\": {\"exclude\":[],\"include\":[]} but could be "warn_error_options": {"exclude": [], "include": []}

@jared-rimmer how exactly are you going about inspecting the value of invocation_arg_dict? I'm trying to inspect the output of 1.4 to write a test for 1.5 and 1.6, but am finding it difficult to actually ensure the proper initial state is setup 🙃

[jtcohen6]

I.e. it doesn't need to be "warn_error_options\": {\"exclude\":[],\"include\":[]} but could be "warn_error_options": {"exclude": [], "include": []}

I agree with this! Feels much more desirable to expose the information in this way