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.

Sign up for GitHub

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

Jump to bottom

Fix #6497: Support global flags passed in after subcommands #8670

Merged
merged 5 commits into from
Sep 26, 2023
Merged

Fix #6497: Support global flags passed in after subcommands #8670

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb305a1
Fix #6497: Support global flags passed in after subcommands
aranke Sep 19, 2023
800c6b1
Don't put click.pass_context in global_flags
aranke Sep 19, 2023
9f014bc
missed a spot
aranke Sep 19, 2023
efdedc3
move a few flags out of global
aranke Sep 19, 2023
913b1cd
Merge branch 'main' into fix_6497
aranke Sep 25, 2023
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-20230919-140514.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Support global flags passed in after subcommands
time: 2023-09-19T14:05:14.600303+01:00
custom:
Author: aranke
Issue: "6497"
106 changes: 62 additions & 44 deletions core/dbt/cli/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import functools
from copy import copy
from dataclasses import dataclass
from typing import Callable, List, Optional, Union
Expand Down Expand Up @@ -118,6 +119,44 @@ def invoke(self, args: List[str], **kwargs) -> dbtRunnerResult:
)


# approach from https://github.com/pallets/click/issues/108#issuecomment-280489786
def global_flags(func):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is neat!

@p.cache_selected_only
@p.debug
@p.deprecated_print
@p.enable_legacy_logger
@p.fail_fast
@p.log_cache_events
@p.log_file_max_bytes
@p.log_format_file
@p.log_level
@p.log_level_file
@p.log_path
@p.macro_debugging
@p.partial_parse
@p.partial_parse_file_path
@p.partial_parse_file_diff
@p.populate_cache
@p.print
@p.printer_width
@p.quiet
@p.record_timing_info
@p.send_anonymous_usage_stats
@p.single_threaded
@p.static_parser
@p.use_colors
@p.use_colors_file
@p.use_experimental_parser
@p.version
@p.version_check
@p.write_json
@functools.wraps(func)
def wrapper(*args, **kwargs):
return func(*args, **kwargs)

return wrapper


# dbt
@click.group(
context_settings={"help_option_names": ["-h", "--help"]},
Expand All @@ -126,38 +165,10 @@ def invoke(self, args: List[str], **kwargs) -> dbtRunnerResult:
epilog="Specify one of these sub-commands and you can find more help from there.",
)
@click.pass_context
@p.cache_selected_only
@p.debug
@p.deprecated_print
@p.enable_legacy_logger
@p.fail_fast
@p.log_cache_events
@p.log_file_max_bytes
@p.log_format
@p.log_format_file
@p.log_level
@p.log_level_file
@p.log_path
@p.macro_debugging
@p.partial_parse
@p.partial_parse_file_path
@p.partial_parse_file_diff
@p.populate_cache
@p.print
@p.printer_width
@p.quiet
@p.record_timing_info
@p.send_anonymous_usage_stats
@p.single_threaded
@p.static_parser
@p.use_colors
@p.use_colors_file
@p.use_experimental_parser
@p.version
@p.version_check
@global_flags
@p.warn_error
@p.warn_error_options
@p.write_json
@p.log_format
def cli(ctx, **kwargs):
"""An ELT tool for managing your SQL transformations and data models.
For more documentation on these commands, visit: docs.getdbt.com
Expand All @@ -167,10 +178,10 @@ def cli(ctx, **kwargs):
# dbt build
@cli.command("build")
@click.pass_context
@global_flags
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice change! What happens when a flag in global flags got passed in twice at different level with different value, which one wins?

Copy link
Member Author

Choose a reason for hiding this comment

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

We throw an error, similar to existing behavior.

Product guidance from jerco: #6497 (comment)

Test:

dbt-core/tests/unit/test_cli_flags.py

Lines 352 to 357 in 913b1cd

def test_duplicate_flags_raises_error(self):
parent_context = self.make_dbt_context("parent", ["--version-check"])
context = self.make_dbt_context("child", ["--version-check"], parent_context)
with pytest.raises(DbtUsageException):
Flags(context)

@p.defer
@p.deprecated_defer
@p.exclude
@p.fail_fast
@p.favor_state
@p.deprecated_favor_state
@p.full_refresh
Expand All @@ -190,7 +201,6 @@ def cli(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -213,6 +223,7 @@ def build(ctx, **kwargs):
# dbt clean
@cli.command("clean")
@click.pass_context
@global_flags
@p.profile
@p.profiles_dir
@p.project_dir
Expand All @@ -235,13 +246,15 @@ def clean(ctx, **kwargs):
# dbt docs
@cli.group()
@click.pass_context
@global_flags
def docs(ctx, **kwargs):
"""Generate or serve the documentation website for your project"""


# dbt docs generate
@docs.command("generate")
@click.pass_context
@global_flags
@p.compile_docs
@p.defer
@p.deprecated_defer
Expand All @@ -261,7 +274,6 @@ def docs(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -284,6 +296,7 @@ def docs_generate(ctx, **kwargs):
# dbt docs serve
@docs.command("serve")
@click.pass_context
@global_flags
@p.browser
@p.port
@p.profile
Expand Down Expand Up @@ -312,6 +325,7 @@ def docs_serve(ctx, **kwargs):
# dbt compile
@cli.command("compile")
@click.pass_context
@global_flags
@p.defer
@p.deprecated_defer
@p.exclude
Expand All @@ -335,7 +349,6 @@ def docs_serve(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -359,6 +372,7 @@ def compile(ctx, **kwargs):
# dbt show
@cli.command("show")
@click.pass_context
@global_flags
@p.defer
@p.deprecated_defer
@p.exclude
Expand All @@ -382,7 +396,6 @@ def compile(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -406,14 +419,14 @@ def show(ctx, **kwargs):
# dbt debug
@cli.command("debug")
@click.pass_context
@global_flags
@p.debug_connection
@p.config_dir
@p.profile
@p.profiles_dir_exists_false
@p.project_dir
@p.target
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
def debug(ctx, **kwargs):
Expand All @@ -432,6 +445,7 @@ def debug(ctx, **kwargs):
# dbt deps
@cli.command("deps")
@click.pass_context
@global_flags
@p.profile
@p.profiles_dir_exists_false
@p.project_dir
Expand All @@ -452,6 +466,7 @@ def deps(ctx, **kwargs):
# dbt init
@cli.command("init")
@click.pass_context
@global_flags
# for backwards compatibility, accept 'project_name' as an optional positional argument
@click.argument("project_name", required=False)
@p.profile
Expand All @@ -474,6 +489,7 @@ def init(ctx, **kwargs):
# dbt list
@cli.command("list")
@click.pass_context
@global_flags
@p.exclude
@p.indirect_selection
@p.models
Expand Down Expand Up @@ -519,14 +535,14 @@ def list(ctx, **kwargs):
# dbt parse
@cli.command("parse")
@click.pass_context
@global_flags
@p.profile
@p.profiles_dir
@p.project_dir
@p.target
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -543,12 +559,12 @@ def parse(ctx, **kwargs):
# dbt run
@cli.command("run")
@click.pass_context
@global_flags
@p.defer
@p.deprecated_defer
@p.favor_state
@p.deprecated_favor_state
@p.exclude
@p.fail_fast
@p.full_refresh
@p.profile
@p.profiles_dir
Expand All @@ -562,7 +578,6 @@ def parse(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -585,14 +600,14 @@ def run(ctx, **kwargs):
# dbt retry
@cli.command("retry")
@click.pass_context
@global_flags
@p.project_dir
@p.profiles_dir
@p.vars
@p.profile
@p.target
@p.state
@p.threads
@p.fail_fast
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -615,6 +630,7 @@ def retry(ctx, **kwargs):
# dbt clone
@cli.command("clone")
@click.pass_context
@global_flags
@p.defer_state
@p.exclude
@p.full_refresh
Expand All @@ -629,7 +645,6 @@ def retry(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.preflight
@requires.profile
@requires.project
Expand All @@ -652,6 +667,7 @@ def clone(ctx, **kwargs):
# dbt run operation
@cli.command("run-operation")
@click.pass_context
@global_flags
@click.argument("macro")
@p.args
@p.profile
Expand Down Expand Up @@ -683,6 +699,7 @@ def run_operation(ctx, **kwargs):
# dbt seed
@cli.command("seed")
@click.pass_context
@global_flags
@p.exclude
@p.full_refresh
@p.profile
Expand All @@ -698,7 +715,6 @@ def run_operation(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand All @@ -720,6 +736,7 @@ def seed(ctx, **kwargs):
# dbt snapshot
@cli.command("snapshot")
@click.pass_context
@global_flags
@p.defer
@p.deprecated_defer
@p.exclude
Expand Down Expand Up @@ -759,13 +776,15 @@ def snapshot(ctx, **kwargs):
# dbt source
@cli.group()
@click.pass_context
@global_flags
def source(ctx, **kwargs):
"""Manage your project's sources"""


# dbt source freshness
@source.command("freshness")
@click.pass_context
@global_flags
@p.exclude
@p.output_path # TODO: Is this ok to re-use? We have three different output params, how much can we consolidate?
@p.profile
Expand Down Expand Up @@ -808,10 +827,10 @@ def freshness(ctx, **kwargs):
# dbt test
@cli.command("test")
@click.pass_context
@global_flags
@p.defer
@p.deprecated_defer
@p.exclude
@p.fail_fast
@p.favor_state
@p.deprecated_favor_state
@p.indirect_selection
Expand All @@ -828,7 +847,6 @@ def freshness(ctx, **kwargs):
@p.target_path
@p.threads
@p.vars
@p.version_check
@requires.postflight
@requires.preflight
@requires.profile
Expand Down
Loading
Loading