-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Deprecate more env vars #7175
Deprecate more env vars #7175
Conversation
8f51e7e
to
a57266a
Compare
def assert_deprecated(self, logs_dir, old_env_var, new_env_var, command="run", old_val="0"): | ||
os.environ[old_env_var] = old_val | ||
run_dbt([command]) | ||
|
||
# replacing new lines with spaces accounts for text wrapping | ||
log_file = read_file(logs_dir, "dbt.log").replace("\n", " ").replace("\\n", " ") | ||
dep_str = f"The environment variable `{old_env_var}` has been renamed as `{new_env_var}`" | ||
|
||
try: | ||
assert dep_str in log_file | ||
except Exception as e: | ||
del os.environ[old_env_var] | ||
raise e | ||
del os.environ[old_env_var] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not so sure about how I handle setting/unsetting env vars here. I am welcome to other suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Do we need to do anything for single_threaded
? Asking because it was listed in the original issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
resolves #6903
Description
Deprecate the rest of the environment variables outlined in the issue.
Checklist
changie new
to create a changelog entry