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

[#2990] Normalize global CLI args/flags #3839

Merged
merged 1 commit into from
Sep 22, 2021
Merged

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Aug 31, 2021

resolves #2990

Description

Make global CLI arguments/flags consistent by allowing them to be set 1) as a command line argument (before subcommands), 2) as an environment variable 3) in the config part of the profile.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Aug 31, 2021
@gshank gshank marked this pull request as draft August 31, 2021 19:43
@gshank
Copy link
Contributor Author

gshank commented Aug 31, 2021

Removed: strict, use_cache, test_new_parser.

Added command line argument before subcommands: --no-version-check, --fail-fast, --profiles_dir.

Global CLI flags: use_experimental_parser, debug, log_format, write_json, partial_parse, use_colors, send_anonymous_user_stats, no_version_checks, fail_fast, profiles_dir, printer_width.

All environment variables start with DBT_, like DBT_PROFILES_DIR.

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Really solid start! Excited to clean up our docs for this :)

FAIL_FAST = get_flag_value('FAIL_FAST', args, user_config)
SEND_ANONYMOUS_USAGE_STATS = get_flag_value('SEND_ANONYMOUS_USAGE_STATS', args, user_config)
PRINTER_WIDTH = get_flag_value('PRINTER_WIDTH', args, user_config)

Copy link
Contributor

Choose a reason for hiding this comment

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

Love how organized + centralized these all are now

core/dbt/flags.py Show resolved Hide resolved
@gshank gshank force-pushed the 2990_global_cli_flags branch 2 times, most recently from 2956b6a to f48faa0 Compare September 1, 2021 18:19
@gshank gshank force-pushed the 2990_global_cli_flags branch 2 times, most recently from 36350bb to 1339ef6 Compare September 13, 2021 18:05
@gshank
Copy link
Contributor Author

gshank commented Sep 13, 2021

I've merged the flags with the 'args' that show up in run_results.json. I removed the 'profiles_dir' attribute from UserConfig. Switched to using flags.VERSION_CHECK instead of args. etc etc. Going to review the flags to make sure they're all actually being used and that there are tests, but hopefully functionality is done.

@gshank gshank force-pushed the 2990_global_cli_flags branch 4 times, most recently from 999bf10 to 99a3b99 Compare September 13, 2021 22:39
@gshank gshank marked this pull request as ready for review September 14, 2021 16:31
Copy link
Contributor

@nathaniel-may nathaniel-may left a comment

Choose a reason for hiding this comment

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

Oh wow. I didn't realize these were so all over the place. Looks like a solid improvement to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More consistency for global CLI flags
3 participants