Skip to content

Commit

Permalink
[#2990] Normalize global CLI args/flags
Browse files Browse the repository at this point in the history
  • Loading branch information
gshank committed Sep 13, 2021
1 parent 3effade commit 68e82e4
Show file tree
Hide file tree
Showing 52 changed files with 654 additions and 513 deletions.
4 changes: 1 addition & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- to simplify config (default usage of `copy_materialization='table'` if is is not found in global or local config)
- to let copy several source tables into single target table at a time. ([Google doc reference](https://cloud.google.com/bigquery/docs/managing-tables#copying_multiple_source_tables))
- Customize ls task JSON output by adding new flag `--output-keys` ([#3778](https://github.com/dbt-labs/dbt/issues/3778), [#3395](https://github.com/dbt-labs/dbt/issues/3395))
- Normalize global CLI arguments/flags ([#2990](https://github.com/dbt-labs/dbt/issues/2990), [#3839](https://github.com/dbt-labs/dbt/pull/3839))

### Fixes

Expand All @@ -15,9 +16,6 @@
- Fix issue when running the `deps` task after the `list` task in the RPC server ([#3846](https://github.com/dbt-labs/dbt/issues/3846), [#3848](https://github.com/dbt-labs/dbt/pull/3848), [#3850](https://github.com/dbt-labs/dbt/pull/3850))
- Fix bug with initializing a dataclass that inherits from `typing.Protocol`, specifically for `dbt.config.profile.Profile` ([#3843](https://github.com/dbt-labs/dbt/issues/3843), [#3855](https://github.com/dbt-labs/dbt/pull/3855))
- Introduce a macro, `get_where_subquery`, for tests that use `where` config. Alias filtering subquery as `dbt_subquery` instead of resource identifier ([#3857](https://github.com/dbt-labs/dbt/issues/3857), [#3859](https://github.com/dbt-labs/dbt/issues/3859))

### Fixes

- Separated table vs view configuration for BigQuery since some configuration is not possible to set for tables vs views. ([#3682](https://github.com/dbt-labs/dbt/issues/3682), [#3691](https://github.com/dbt-labs/dbt/issues/3682))

### Under the hood
Expand Down
12 changes: 0 additions & 12 deletions core/dbt/adapters/base/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,6 @@ def _close_handle(cls, connection: Connection) -> None:
@classmethod
def _rollback(cls, connection: Connection) -> None:
"""Roll back the given connection."""
if flags.STRICT_MODE:
if not isinstance(connection, Connection):
raise dbt.exceptions.CompilerException(
f'In _rollback, got {connection} - not a Connection!'
)

if connection.transaction_open is False:
raise dbt.exceptions.InternalException(
f'Tried to rollback transaction on connection '
Expand All @@ -257,12 +251,6 @@ def _rollback(cls, connection: Connection) -> None:

@classmethod
def close(cls, connection: Connection) -> Connection:
if flags.STRICT_MODE:
if not isinstance(connection, Connection):
raise dbt.exceptions.CompilerException(
f'In close, got {connection} - not a Connection!'
)

# if the connection is in closed or init, there's nothing to do
if connection.state in {ConnectionState.CLOSED, ConnectionState.INIT}:
return connection
Expand Down
20 changes: 4 additions & 16 deletions core/dbt/adapters/base/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
get_relation_returned_multiple_results,
InternalException, NotImplementedException, RuntimeException,
)
from dbt import flags

from dbt import deprecations
from dbt.adapters.protocol import (
Expand Down Expand Up @@ -289,9 +288,7 @@ def clear_macro_manifest(self):
def _schema_is_cached(self, database: Optional[str], schema: str) -> bool:
"""Check if the schema is cached, and by default logs if it is not."""

if flags.USE_CACHE is False:
return False
elif (database, schema) not in self.cache:
if (database, schema) not in self.cache:
logger.debug(
'On "{}": cache miss for schema "{}.{}", this is inefficient'
.format(self.nice_connection_name(), database, schema)
Expand Down Expand Up @@ -340,9 +337,6 @@ def _relations_cache_for_schemas(self, manifest: Manifest) -> None:
"""Populate the relations cache for the given schemas. Returns an
iterable of the schemas populated, as strings.
"""
if not flags.USE_CACHE:
return

cache_schemas = self._get_cache_schemas(manifest)
with executor(self.config) as tpe:
futures: List[Future[List[BaseRelation]]] = []
Expand Down Expand Up @@ -375,9 +369,6 @@ def set_relations_cache(
"""Run a query that gets a populated cache of the relations in the
database and set the cache on this adapter.
"""
if not flags.USE_CACHE:
return

with self.cache.lock:
if clear:
self.cache.clear()
Expand All @@ -391,8 +382,7 @@ def cache_added(self, relation: Optional[BaseRelation]) -> str:
raise_compiler_error(
'Attempted to cache a null relation for {}'.format(name)
)
if flags.USE_CACHE:
self.cache.add(relation)
self.cache.add(relation)
# so jinja doesn't render things
return ''

Expand All @@ -406,8 +396,7 @@ def cache_dropped(self, relation: Optional[BaseRelation]) -> str:
raise_compiler_error(
'Attempted to drop a null relation for {}'.format(name)
)
if flags.USE_CACHE:
self.cache.drop(relation)
self.cache.drop(relation)
return ''

@available
Expand All @@ -428,8 +417,7 @@ def cache_renamed(
.format(src_name, dst_name, name)
)

if flags.USE_CACHE:
self.cache.rename(from_relation, to_relation)
self.cache.rename(from_relation, to_relation)
return ''

###
Expand Down
14 changes: 0 additions & 14 deletions core/dbt/adapters/sql/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
Connection, ConnectionState, AdapterResponse
)
from dbt.logger import GLOBAL_LOGGER as logger
from dbt import flags


class SQLConnectionManager(BaseConnectionManager):
Expand Down Expand Up @@ -144,13 +143,6 @@ def add_commit_query(self):

def begin(self):
connection = self.get_thread_connection()

if flags.STRICT_MODE:
if not isinstance(connection, Connection):
raise dbt.exceptions.CompilerException(
f'In begin, got {connection} - not a Connection!'
)

if connection.transaction_open is True:
raise dbt.exceptions.InternalException(
'Tried to begin a new transaction on connection "{}", but '
Expand All @@ -163,12 +155,6 @@ def begin(self):

def commit(self):
connection = self.get_thread_connection()
if flags.STRICT_MODE:
if not isinstance(connection, Connection):
raise dbt.exceptions.CompilerException(
f'In commit, got {connection} - not a Connection!'
)

if connection.transaction_open is False:
raise dbt.exceptions.InternalException(
'Tried to commit transaction on connection "{}", but '
Expand Down
1 change: 1 addition & 0 deletions core/dbt/config/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from .renderer import ProfileRenderer

DEFAULT_THREADS = 1
# This is where PROFILES_DIR is initially set. Override in main.py.
DEFAULT_PROFILES_DIR = os.path.join(os.path.expanduser('~'), '.dbt')
PROFILES_DIR = os.path.expanduser(
os.getenv('DBT_PROFILES_DIR', DEFAULT_PROFILES_DIR)
Expand Down
5 changes: 3 additions & 2 deletions core/dbt/config/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from .project import Project
from .renderer import DbtProjectYamlRenderer, ProfileRenderer
from .utils import parse_cli_vars
from dbt import flags
from dbt import tracking
from dbt.adapters.factory import get_relation_class_by_name, get_include_paths
from dbt.helper_types import FQNPath, PathSet
Expand Down Expand Up @@ -144,7 +145,7 @@ def new_project(self, project_root: str) -> 'RuntimeConfig':
project = Project.from_project_root(
project_root,
renderer,
verify_version=getattr(self.args, 'version_check', False),
verify_version=bool(flags.VERSION_CHECK),
)

cfg = self.from_parts(
Expand Down Expand Up @@ -197,7 +198,7 @@ def collect_parts(
) -> Tuple[Project, Profile]:
# profile_name from the project
project_root = args.project_dir if args.project_dir else os.getcwd()
version_check = getattr(args, 'version_check', False)
version_check = bool(flags.VERSION_CHECK)
partial = Project.partial_load(
project_root,
verify_version=version_check
Expand Down
2 changes: 0 additions & 2 deletions core/dbt/context/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,6 @@ def flags(self) -> Any:
The list of valid flags are:
- `flags.STRICT_MODE`: True if `--strict` (or `-S`) was provided on the
command line
- `flags.FULL_REFRESH`: True if `--full-refresh` was provided on the
command line
- `flags.NON_DESTRUCTIVE`: True if `--non-destructive` was provided on
Expand Down
3 changes: 0 additions & 3 deletions core/dbt/contracts/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,6 @@ class UserConfigContract(Protocol):
partial_parse: Optional[bool] = None
printer_width: Optional[int] = None

def set_values(self, cookie_dir: str) -> None:
...


class HasCredentials(Protocol):
credentials: Credentials
Expand Down
4 changes: 1 addition & 3 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,7 @@ def __post_init__(self):
self.user_id = tracking.active_user.id

if self.send_anonymous_usage_stats is None:
self.send_anonymous_usage_stats = (
not tracking.active_user.do_not_track
)
self.send_anonymous_usage_stats = flags.SEND_ANONYMOUS_USAGE_STATS

@classmethod
def default(cls):
Expand Down
12 changes: 0 additions & 12 deletions core/dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,6 @@ def patch(self, patch: 'ParsedNodePatch'):
self.columns = patch.columns
self.meta = patch.meta
self.docs = patch.docs
if flags.STRICT_MODE:
# It seems odd that an instance can be invalid
# Maybe there should be validation or restrictions
# elsewhere?
assert isinstance(self, dbtClassMixin)
dct = self.to_dict(omit_none=False)
self.validate(dct)

def get_materialization(self):
return self.config.materialized
Expand Down Expand Up @@ -509,11 +502,6 @@ def patch(self, patch: ParsedMacroPatch):
self.meta = patch.meta
self.docs = patch.docs
self.arguments = patch.arguments
if flags.STRICT_MODE:
# What does this actually validate?
assert isinstance(self, dbtClassMixin)
dct = self.to_dict(omit_none=False)
self.validate(dct)

def same_contents(self, other: Optional['ParsedMacro']) -> bool:
if other is None:
Expand Down
23 changes: 8 additions & 15 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
from dbt.contracts.util import Replaceable, Mergeable, list_str
from dbt.contracts.connection import UserConfigContract, QueryComment
from dbt.contracts.connection import QueryComment, UserConfigContract
from dbt.helper_types import NoValue
from dbt.logger import GLOBAL_LOGGER as logger # noqa
from dbt import tracking
from dbt import ui
from dbt.dataclass_schema import (
dbtClassMixin, ValidationError,
HyphenatedDbtClassMixin,
Expand Down Expand Up @@ -230,18 +228,13 @@ class UserConfig(ExtensibleDbtClassMixin, Replaceable, UserConfigContract):
use_colors: Optional[bool] = None
partial_parse: Optional[bool] = None
printer_width: Optional[int] = None

def set_values(self, cookie_dir):
if self.send_anonymous_usage_stats:
tracking.initialize_tracking(cookie_dir)
else:
tracking.do_not_track()

if self.use_colors is not None:
ui.use_colors(self.use_colors)

if self.printer_width:
ui.printer_width(self.printer_width)
write_json: Optional[bool] = None
warn_error: Optional[bool] = None
log_format: Optional[bool] = None
debug: Optional[bool] = None
version_check: Optional[bool] = None
fail_fast: Optional[bool] = None
use_experimental_parser: Optional[bool] = None


@dataclass
Expand Down
Loading

0 comments on commit 68e82e4

Please sign in to comment.