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 adapter.dispatch #2679

Merged
merged 4 commits into from
Aug 4, 2020
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
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

### Breaking changes
- `adapter_macro` is no longer a macro, instead it is a builtin context method. Any custom macros that intercepted it by going through `context['dbt']` will need to instead access it via `context['builtins']` ([#2302](https://github.com/fishtown-analytics/dbt/issues/2302), [#2673](https://github.com/fishtown-analytics/dbt/pull/2673))
- `adapter_macro` is now deprecated. Use `adapter.dispatch` instead.

### Features
- Added a `dispatch` method to the context adapter and deprecated `adapter_macro`. ([#2302](https://github.com/fishtown-analytics/dbt/issues/2302), [#2679](https://github.com/fishtown-analytics/dbt/pull/2679))

## dbt 0.18.0b2 (July 30, 2020)

Expand Down Expand Up @@ -123,11 +127,9 @@ Contributors:
- dbt compile and ls no longer create schemas if they don't already exist ([#2525](https://github.com/fishtown-analytics/dbt/issues/2525), [#2528](https://github.com/fishtown-analytics/dbt/pull/2528))
- `dbt deps` now respects the `--project-dir` flag, so using `dbt deps --project-dir=/some/path` and then `dbt run --project-dir=/some/path` will properly find dependencies ([#2519](https://github.com/fishtown-analytics/dbt/issues/2519), [#2534](https://github.com/fishtown-analytics/dbt/pull/2534))
- `packages.yml` revision/version fields can be float-like again (`revision: '1.0'` is valid). ([#2518](https://github.com/fishtown-analytics/dbt/issues/2518), [#2535](https://github.com/fishtown-analytics/dbt/pull/2535))
<<<<<<< HEAD
- dbt again respects config aliases in config() calls ([#2557](https://github.com/fishtown-analytics/dbt/issues/2557), [#2559](https://github.com/fishtown-analytics/dbt/pull/2559))


=======
- Parallel RPC requests no longer step on each others' arguments ([[#2484](https://github.com/fishtown-analytics/dbt/issues/2484), [#2554](https://github.com/fishtown-analytics/dbt/pull/2554)])
- `persist_docs` now takes into account descriptions for nested columns in bigquery ([#2549](https://github.com/fishtown-analytics/dbt/issues/2549), [#2550](https://github.com/fishtown-analytics/dbt/pull/2550))
- On windows (depending upon OS support), dbt no longer fails with errors when writing artifacts ([#2558](https://github.com/fishtown-analytics/dbt/issues/2558), [#2566](https://github.com/fishtown-analytics/dbt/pull/2566))
Expand All @@ -137,7 +139,6 @@ Contributors:

Contributors:
- [@bodschut](https://github.com/bodschut) ([#2550](https://github.com/fishtown-analytics/dbt/pull/2550))
>>>>>>> dev/0.17.1

## dbt 0.17.0 (June 08, 2020)

Expand Down
125 changes: 84 additions & 41 deletions core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@
)
from typing_extensions import Protocol


from dbt import deprecations
from dbt.adapters.base.column import Column
from dbt.adapters.factory import get_adapter, get_adapter_package_names
from dbt.clients import agate_helper
from dbt.clients.jinja import get_rendered
from dbt.clients.jinja import get_rendered, MacroGenerator
from dbt.config import RuntimeConfig, Project
from .base import contextmember, contextproperty, Var
from .configured import FQNLookup
from .context_config import ContextConfigType
from .macros import MacroNamespaceBuilder
from .macros import MacroNamespaceBuilder, MacroNamespace
from .manifest import ManifestContext
from dbt.contracts.graph.manifest import Manifest, Disabled
from dbt.contracts.graph.compiled import (
Expand Down Expand Up @@ -82,9 +82,10 @@ class BaseDatabaseWrapper:
Wrapper for runtime database interaction. Applies the runtime quote policy
via a relation proxy.
"""
def __init__(self, adapter):
def __init__(self, adapter, namespace: MacroNamespace):
self.adapter = adapter
self.Relation = RelationProxy(adapter)
self.namespace = namespace

def __getattr__(self, name):
raise NotImplementedError('subclasses need to implement this')
Expand All @@ -99,6 +100,67 @@ def type(self):
def commit(self):
return self.adapter.commit_if_has_connection()

def _get_adapter_macro_prefixes(self) -> List[str]:
# a future version of this could have plugins automatically call fall
# back to their dependencies' dependencies by using
# `get_adapter_type_names` instead of `[self.config.credentials.type]`
search_prefixes = [self.adapter.type(), 'default']
return search_prefixes

def dispatch(
self, macro_name: str, packages: Optional[List[str]] = None
) -> MacroGenerator:
search_packages: List[Optional[str]]

if '.' in macro_name:
suggest_package, suggest_macro_name = macro_name.split('.', 1)
msg = (
f'In adapter.dispatch, got a macro name of "{macro_name}", '
f'but "." is not a valid macro name component. Did you mean '
f'`adapter.dispatch("{suggest_macro_name}", '
f'packages=["{suggest_package}"])`?'
)
raise CompilationException(msg)

if packages is None:
search_packages = [None]
elif isinstance(packages, str):
raise CompilationException(
f'In adapter.dispatch, got a string packages argument '
f'("{packages}"), but packages should be None or a list.'
)
else:
search_packages = packages

attempts = []

for package_name in search_packages:
for prefix in self._get_adapter_macro_prefixes():
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I right in understanding that the order of preference is packages > adapter specificity? So e.g. a dispatch with two packages running on Postgres would look for:

  1. packages[0], postgres_
  2. packages[0], default__
  3. packages[1], postgres__
  4. packages[1], default__

Per the comment above, someday we may make it so that running on Redshift looks for:

  1. packages[0], redshift__
  2. packages[0], postgres__
  3. packages[0], default__
  4. packages[1], redshift__
    ...

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. We can of course swap this order if we want, but it seemed more correct to me to go by package first - looking at the example in #2302, it seems that you'd want your custom macro to tell dbt-utils "search this other package first and then fall back to dbt-utils if it's not there". This provides a nice escape hatch for issues where "dbt-utils has an implementation of a macro that doesn't work for me because of my obscure workflow". Or even just a bug!

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree! Give first priority to the place where end users have the most control, let them override default__ or do anything they want. Just wanted to make sure I had this down before writing docs

search_name = f'{prefix}__{macro_name}'
try:
macro = self.namespace.get_from_package(
package_name, search_name
)
except CompilationException as exc:
raise CompilationException(
f'In dispatch: {exc.msg}',
) from exc

if package_name is None:
attempts.append(search_name)
else:
attempts.append(f'{package_name}.{search_name}')

if macro is not None:
return macro

searched = ', '.join(repr(a) for a in attempts)
msg = (
f"In dispatch: No macro named '{macro_name}' found\n"
f" Searched for: {searched}"
)
raise CompilationException(msg)


class BaseResolver(metaclass=abc.ABCMeta):
def __init__(self, db_wrapper, model, config, manifest):
Expand Down Expand Up @@ -574,7 +636,9 @@ def __init__(
self.context_config: Optional[ContextConfigType] = context_config
self.provider: Provider = provider
self.adapter = get_adapter(self.config)
self.db_wrapper = self.provider.DatabaseWrapper(self.adapter)
self.db_wrapper = self.provider.DatabaseWrapper(
self.adapter, self.namespace
)

def _get_namespace_builder(self):
internal_packages = get_adapter_package_names(
Expand Down Expand Up @@ -1045,13 +1109,6 @@ def sql(self) -> Optional[str]:
def sql_now(self) -> str:
return self.adapter.date_function()

def _get_adapter_macro_prefixes(self) -> List[str]:
# a future version of this could have plugins automatically call fall
# back to their dependencies' dependencies by using
# `get_adapter_type_names` instead of `[self.config.credentials.type]`
search_prefixes = [self.config.credentials.type, 'default']
return search_prefixes

@contextmember
def adapter_macro(self, name: str, *args, **kwargs):
"""Find the most appropriate macro for the name, considering the
Expand Down Expand Up @@ -1096,38 +1153,24 @@ def adapter_macro(self, name: str, *args, **kwargs):
...
{%- endmacro %}
"""
original_name: str = name
package_name: Optional[str] = None
deprecations.warn('adapter-macro', macro_name=name)
original_name = name
package_names: Optional[List[str]] = None
if '.' in name:
package_name, name = name.split('.', 1)
package_names = [package_name]

attempts = []

for prefix in self._get_adapter_macro_prefixes():
search_name = f'{prefix}__{name}'
try:
macro = self.namespace.get_from_package(
package_name, search_name
)
except CompilationException as exc:
raise CompilationException(
f'In adapter_macro: {exc.msg}, original name '
f"'{original_name}'",
node=self.model,
) from exc
if package_name is None:
attempts.append(search_name)
else:
attempts.append(f'{package_name}.{search_name}')
if macro is not None:
return macro(*args, **kwargs)

searched = ', '.join(repr(a) for a in attempts)
raise_compiler_error(
f"In adapter_macro: No macro named '{name}' found\n"
f" Original name: '{original_name}'\n"
f" Searched for: {searched}"
)
try:
macro = self.db_wrapper.dispatch(
macro_name=name, packages=package_names
)
except CompilationException as exc:
raise CompilationException(
f'In adapter_macro: {exc.msg}\n'
f" Original name: '{original_name}'",
node=self.model
) from exc
return macro(*args, **kwargs)


class MacroContext(ProviderContext):
Expand Down
10 changes: 10 additions & 0 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ class ExecuteMacrosReleaseDeprecation(DBTDeprecation):
'''


class AdapterMacroDeprecation(DBTDeprecation):
_name = 'adapter-macro'
_description = '''\
The "adapter_macro" macro has been deprecated. Instead, use the
`adapter.dispatch` method to find a macro and call the result.
adapter_macro was called for: {macro_name}
'''


_adapter_renamed_description = """\
The adapter function `adapter.{old_name}` is deprecated and will be removed in
a future release of dbt. Please use `adapter.{new_name}` instead.
Expand Down Expand Up @@ -160,6 +169,7 @@ def warn(name, *args, **kwargs):
ModelsKeyNonModelDeprecation(),
DbtProjectYamlDeprecation(),
ExecuteMacrosReleaseDeprecation(),
AdapterMacroDeprecation(),
]

deprecations: Dict[str, DBTDeprecation] = {
Expand Down
Loading