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

Create method for env var deprecation #7086

Merged
merged 5 commits into from
Mar 8, 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
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20230224-132811.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Add deprecation warning for DBT_NO_PRINT
time: 2023-02-24T13:28:11.295561-06:00
custom:
Author: stu-k
Issue: "6960"
71 changes: 63 additions & 8 deletions core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
from importlib import import_module
from multiprocessing import get_context
from pprint import pformat as pf
from typing import Set, List
from typing import Callable, Dict, List, Set

from click import Context, get_current_context, BadOptionUsage
from click.core import ParameterSource, Command, Group

from dbt.config.profile import read_user_config
from dbt.contracts.project import UserConfig
from dbt.deprecations import renamed_env_var
from dbt.helper_types import WarnErrorOptions
from dbt.cli.resolvers import default_project_dir, default_log_path

Expand Down Expand Up @@ -76,6 +77,11 @@ def args_to_context(args: List[str]) -> Context:
return sub_command_ctx


DEPRECATED_PARAMS = {
"deprecated_print": "print",
}
Comment on lines +80 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the approach here be extensible to other deprecated/renamed flags & env vars? (#6903)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.



@dataclass(frozen=True)
class Flags:
def __init__(self, ctx: Context = None, user_config: UserConfig = None) -> None:
Expand All @@ -87,7 +93,7 @@ def __init__(self, ctx: Context = None, user_config: UserConfig = None) -> None:
if ctx is None:
ctx = get_current_context()

def assign_params(ctx, params_assigned_from_default):
def assign_params(ctx, params_assigned_from_default, deprecated_env_vars):
"""Recursively adds all click params to flag object"""
for param_name, param_value in ctx.params.items():
# TODO: this is to avoid duplicate params being defined in two places (version_check in run and cli)
Expand All @@ -97,6 +103,10 @@ def assign_params(ctx, params_assigned_from_default):
# when using frozen dataclasses.
# https://docs.python.org/3/library/dataclasses.html#frozen-instances
if hasattr(self, param_name.upper()):
if param_name in deprecated_env_vars:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to consider a case where people set both deprecated and and the new env var?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is that case. The warning message states that the deprecated env var will always be respected over the flag and if you want to respect the flag you should unset it.

# param already set via its deprecated but still respected env var
continue

if param_name not in EXPECTED_DUPLICATE_PARAMS:
raise Exception(
f"Duplicate flag names found in click command: {param_name}"
Expand All @@ -107,15 +117,58 @@ def assign_params(ctx, params_assigned_from_default):
if ctx.get_parameter_source(param_name) != ParameterSource.DEFAULT:
object.__setattr__(self, param_name.upper(), param_value)
else:
object.__setattr__(self, param_name.upper(), param_value)
if ctx.get_parameter_source(param_name) == ParameterSource.DEFAULT:
params_assigned_from_default.add(param_name)
# handle deprecated env vars while still respecting old values
# e.g. DBT_NO_PRINT -> DBT_PRINT if DBT_NO_PRINT is set, it is
# respected over DBT_PRINT or --print
if param_name in DEPRECATED_PARAMS:

# deprecated env vars can only be set via env var.
# we use the deprecated option in click to serialize the value
# from the env var string
param_source = ctx.get_parameter_source(param_name)
if param_source == ParameterSource.DEFAULT:
continue
elif param_source != ParameterSource.ENVIRONMENT:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually possible since those params are already hidden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't be but I'd rather be explicit about what should happen here.

raise BadOptionUsage(
"Deprecated parameters can only be set via environment variables"
)

# rename for clarity
dep_name = param_name
new_name = DEPRECATED_PARAMS.get(dep_name)

# find param objects for their envvar name
try:
dep_opt = [x for x in ctx.command.params if x.name == dep_name][0]
new_opt = [x for x in ctx.command.params if x.name == new_name][0]
except IndexError:
raise Exception(
f"No deprecated param name match from {dep_name} to {new_name}"
)

# adding the deprecation warning function to the set
deprecated_env_vars[new_name] = renamed_env_var(
old_name=dep_opt.envvar,
new_name=new_opt.envvar,
)

object.__setattr__(self, new_name.upper(), param_value)
else:
object.__setattr__(self, param_name.upper(), param_value)
if ctx.get_parameter_source(param_name) == ParameterSource.DEFAULT:
params_assigned_from_default.add(param_name)

if ctx.parent:
assign_params(ctx.parent, params_assigned_from_default)
assign_params(ctx.parent, params_assigned_from_default, deprecated_env_vars)

params_assigned_from_default = set() # type: Set[str]
assign_params(ctx, params_assigned_from_default)
deprecated_env_vars: Dict[str, Callable] = {}
assign_params(ctx, params_assigned_from_default, deprecated_env_vars)

# set deprecated_env_var_warnings to be fired later after events have been init
object.__setattr__(
self, "deprecated_env_var_warnings", [x for x in deprecated_env_vars.values()]
)

# Get the invoked command flags
invoked_subcommand_name = (
Expand All @@ -126,7 +179,9 @@ def assign_params(ctx, params_assigned_from_default):
invoked_subcommand.allow_extra_args = True
invoked_subcommand.ignore_unknown_options = True
invoked_subcommand_ctx = invoked_subcommand.make_context(None, sys.argv)
assign_params(invoked_subcommand_ctx, params_assigned_from_default)
assign_params(
invoked_subcommand_ctx, params_assigned_from_default, deprecated_env_vars
)

if not user_config:
profiles_dir = getattr(self, "PROFILES_DIR", None)
Expand Down
1 change: 1 addition & 0 deletions core/dbt/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def invoke(self, args: List[str]) -> Tuple[Optional[List], bool]:
@p.macro_debugging
@p.partial_parse
@p.print
@p.deprecated_print
@p.printer_width
@p.quiet
@p.record_timing_info
Expand Down
28 changes: 17 additions & 11 deletions core/dbt/cli/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@
from dbt.cli.resolvers import default_project_dir, default_profiles_dir
from dbt.version import get_version_information

# 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,
)

args = click.option(
"--args",
Expand Down Expand Up @@ -214,17 +207,23 @@
type=click.INT,
)

# TODO: The env var and name (reflected in flags) are corrections!
# The original name was `NO_PRINT` and used the env var `DBT_NO_PRINT`.
# Both of which break existing naming conventions.
# This will need to be fixed before use in the main codebase and communicated as a change to the community!
# envvar was previously named DBT_NO_PRINT
print = click.option(
"--print/--no-print",
envvar="DBT_PRINT",
help="Output all {{ print() }} macro calls.",
default=True,
)

deprecated_print = click.option(
"--deprecated-print/--deprecated-no-print",
envvar="DBT_NO_PRINT",
help="Internal flag for deprecating old env var.",
default=True,
hidden=True,
callback=lambda ctx, param, value: not value,
)

printer_width = click.option(
"--printer-width",
envvar="DBT_PRINTER_WIDTH",
Expand Down Expand Up @@ -323,6 +322,13 @@
"--selector", envvar=None, help="The selector name to use, as defined in selectors.yml"
)

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,
)

show = click.option(
"--show", envvar=None, help="Show a sample of the loaded data in the terminal", is_flag=True
)
Expand Down
3 changes: 3 additions & 0 deletions core/dbt/cli/requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ def wrapper(*args, **kwargs):
flags_dict_str = cast_dict_to_dict_of_strings(get_flag_dict())
fire_event(MainReportArgs(args=flags_dict_str))

# Deprecation warnings
[dep_fn() for dep_fn in flags.deprecated_env_var_warnings]

if active_user is not None: # mypy appeasement, always true
fire_event(MainTrackingUserState(user_state=active_user.state()))

Expand Down
15 changes: 15 additions & 0 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,21 @@ class ExposureNameDeprecation(DBTDeprecation):
_event = "ExposureNameDeprecation"


def renamed_env_var(old_name: str, new_name: str):
class EnvironmentVariableRenamed(DBTDeprecation):
_name = f"environment-variable-renamed:{old_name}"
_event = "EnvironmentVariableRenamed"

dep = EnvironmentVariableRenamed()
deprecations_list.append(dep)
deprecations[dep.name] = dep

def cb():
dep.show(old_name=old_name, new_name=new_name)

return cb


def warn(name, *args, **kwargs):
if name not in deprecations:
# this should (hopefully) never happen
Expand Down
Binary file modified core/dbt/docs/build/doctrees/environment.pickle
Binary file not shown.
Binary file modified core/dbt/docs/build/doctrees/index.doctree
Binary file not shown.
2 changes: 1 addition & 1 deletion core/dbt/docs/build/html/.buildinfo
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Sphinx build info version 1
# This file hashes the configuration used when building these files. When it is not found, a full rebuild will be done.
config: e27d6c1c419f2f0af393858cdf674109
config: 0d25ef12a43286020bcd8b805064f01c
tags: 645f666f9bcd5a90fca523b33c5a78b7
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*
* _sphinx_javascript_frameworks_compat.js
* ~~~~~~~~~~
*
* Compatability shim for jQuery and underscores.js.
*
* WILL BE REMOVED IN Sphinx 6.0
* xref RemovedInSphinx60Warning
*
*/

/**
* select a different prefix for underscore
*/
$u = _.noConflict();


/**
* small helper function to urldecode strings
*
* See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent#Decoding_query_parameters_from_a_URL
*/
jQuery.urldecode = function(x) {
if (!x) {
return x
}
return decodeURIComponent(x.replace(/\+/g, ' '));
};

/**
* small helper function to urlencode strings
*/
jQuery.urlencode = encodeURIComponent;

/**
* This function returns the parsed url parameters of the
* current request. Multiple values per key are supported,
* it will always return arrays of strings for the value parts.
*/
jQuery.getQueryParameters = function(s) {
if (typeof s === 'undefined')
s = document.location.search;
var parts = s.substr(s.indexOf('?') + 1).split('&');
var result = {};
for (var i = 0; i < parts.length; i++) {
var tmp = parts[i].split('=', 2);
var key = jQuery.urldecode(tmp[0]);
var value = jQuery.urldecode(tmp[1]);
if (key in result)
result[key].push(value);
else
result[key] = [value];
}
return result;
};

/**
* highlight a given string on a jquery object by wrapping it in
* span elements with the given class name.
*/
jQuery.fn.highlightText = function(text, className) {
function highlight(node, addItems) {
if (node.nodeType === 3) {
var val = node.nodeValue;
var pos = val.toLowerCase().indexOf(text);
if (pos >= 0 &&
!jQuery(node.parentNode).hasClass(className) &&
!jQuery(node.parentNode).hasClass("nohighlight")) {
var span;
var isInSVG = jQuery(node).closest("body, svg, foreignObject").is("svg");
if (isInSVG) {
span = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
} else {
span = document.createElement("span");
span.className = className;
}
span.appendChild(document.createTextNode(val.substr(pos, text.length)));
node.parentNode.insertBefore(span, node.parentNode.insertBefore(
document.createTextNode(val.substr(pos + text.length)),
node.nextSibling));
node.nodeValue = val.substr(0, pos);
if (isInSVG) {
var rect = document.createElementNS("http://www.w3.org/2000/svg", "rect");
var bbox = node.parentElement.getBBox();
rect.x.baseVal.value = bbox.x;
rect.y.baseVal.value = bbox.y;
rect.width.baseVal.value = bbox.width;
rect.height.baseVal.value = bbox.height;
rect.setAttribute('class', className);
addItems.push({
"parent": node.parentNode,
"target": rect});
}
}
}
else if (!jQuery(node).is("button, select, textarea")) {
jQuery.each(node.childNodes, function() {
highlight(this, addItems);
});
}
}
var addItems = [];
var result = this.each(function() {
highlight(this, addItems);
});
for (var i = 0; i < addItems.length; ++i) {
jQuery(addItems[i].parent).before(addItems[i].target);
}
return result;
};

/*
* backward compatibility for jQuery.browser
* This will be supported until firefox bug is fixed.
*/
if (!jQuery.browser) {
jQuery.uaMatch = function(ua) {
ua = ua.toLowerCase();

var match = /(chrome)[ \/]([\w.]+)/.exec(ua) ||
/(webkit)[ \/]([\w.]+)/.exec(ua) ||
/(opera)(?:.*version|)[ \/]([\w.]+)/.exec(ua) ||
/(msie) ([\w.]+)/.exec(ua) ||
ua.indexOf("compatible") < 0 && /(mozilla)(?:.*? rv:([\w.]+)|)/.exec(ua) ||
[];

return {
browser: match[ 1 ] || "",
version: match[ 2 ] || "0"
};
};
jQuery.browser = {};
jQuery.browser[jQuery.uaMatch(navigator.userAgent).browser] = true;
}
4 changes: 1 addition & 3 deletions core/dbt/docs/build/html/_static/alabaster.css
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,7 @@ table.footnote td {
}

dl {
margin-left: 0;
margin-right: 0;
margin-top: 0;
margin: 0;
padding: 0;
}

Expand Down
Loading