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

Remove operations [#1117] #1176

Merged
merged 2 commits into from
Dec 18, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 15 additions & 11 deletions core/dbt/adapters/base/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from dbt.adapters.base import BaseRelation
from dbt.adapters.cache import RelationsCache

GET_CATALOG_OPERATION_NAME = 'get_catalog_data'
GET_CATALOG_MACRO_NAME = 'get_catalog'


def _expect_row_value(key, row):
Expand Down Expand Up @@ -655,25 +655,29 @@ def convert_agate_type(cls, agate_table, col_idx):
###
# Operations involving the manifest
###
def run_operation(self, manifest, operation_name):
"""Look the operation identified by operation_name up in the manifest
and run it.
def execute_macro(self, manifest, macro_name, project=None):
"""Look macro_name up in the manifest and execute its results.

Return an an AttrDict with three attributes: 'table', 'data', and
'status'. 'table' is an agate.Table.
"""
operation = manifest.find_operation_by_name(operation_name, 'dbt')
macro = manifest.find_macro_by_name(macro_name, project)
if macro is None:
raise dbt.exceptions.RuntimeException(
'Could not find macro with name {} in project {}'
.format(macro_name, project)
)

# This causes a reference cycle, as dbt.context.runtime.generate()
# ends up calling get_adapter, so the import has to be here.
import dbt.context.runtime
context = dbt.context.runtime.generate(
operation,
macro,
self.config,
manifest,
)

result = operation.generator(context)()
result = macro.generator(context)()
return result

@classmethod
Expand All @@ -684,13 +688,13 @@ def _catalog_filter_table(cls, table, manifest):
return table.where(_catalog_filter_schemas(manifest))

def get_catalog(self, manifest):
"""Get the catalog for this manifest by running the get catalog
operation. Returns an agate.Table of catalog information.
"""Get the catalog for this manifest by running the get catalog macro.
Returns an agate.Table of catalog information.
"""
try:
table = self.run_operation(manifest, GET_CATALOG_OPERATION_NAME)
table = self.execute_macro(manifest, GET_CATALOG_MACRO_NAME)
finally:
self.release_connection(GET_CATALOG_OPERATION_NAME)
self.release_connection(GET_CATALOG_MACRO_NAME)

results = self._catalog_filter_table(table, manifest)
return results
Expand Down
28 changes: 1 addition & 27 deletions core/dbt/clients/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,7 @@ def call(*args, **kwargs):
template = template_cache.get_node_template(node)
module = template.make_module(context, False, context)

if node['resource_type'] == NodeType.Operation:
macro = module.__dict__[dbt.utils.get_dbt_operation_name(name)]
else:
macro = module.__dict__[dbt.utils.get_dbt_macro_name(name)]
macro = module.__dict__[dbt.utils.get_dbt_macro_name(name)]
Copy link
Member

Choose a reason for hiding this comment

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

👍

module.__dict__.update(context)

try:
Expand Down Expand Up @@ -148,28 +145,6 @@ def parse(self, parser):
return node


class OperationExtension(jinja2.ext.Extension):
tags = ['operation']

def parse(self, parser):
node = jinja2.nodes.Macro(lineno=next(parser.stream).lineno)
operation_name = \
parser.parse_assign_target(name_only=True).name

node.args = []
node.defaults = []

while parser.stream.skip_if('comma'):
target = parser.parse_assign_target(name_only=True)

node.name = dbt.utils.get_operation_macro_name(operation_name)

node.body = parser.parse_statements(('name:endoperation',),
drop_needle=True)

return node


Copy link
Member

Choose a reason for hiding this comment

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

👍

class DocumentationExtension(jinja2.ext.Extension):
tags = ['docs']

Expand Down Expand Up @@ -253,7 +228,6 @@ def get_environment(node=None, capture_macros=False):
args['undefined'] = create_macro_capture_env(node)

args['extensions'].append(MaterializationExtension)
args['extensions'].append(OperationExtension)
args['extensions'].append(DocumentationExtension)

return MacroFuzzEnvironment(**args)
Expand Down
32 changes: 12 additions & 20 deletions core/dbt/context/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,22 +393,6 @@ def generate_base(model, model_dict, config, manifest, source_config,
"try_or_compiler_error": try_or_compiler_error(model)
})

# Operations do not represent database relations, so there should be no
# 'this' variable in the context for operations. The Operation branch
# below should be removed in a future release. The fake relation below
# mirrors the historical implementation, without causing errors around
# the missing 'alias' attribute for operations
#
# https://github.com/fishtown-analytics/dbt/issues/878
if model.resource_type == NodeType.Operation:
this = db_wrapper.adapter.Relation.create(
schema=config.credentials.schema,
identifier=model.name
)
else:
this = get_this_relation(db_wrapper, config, model_dict)

context["this"] = this
return context


Expand All @@ -431,9 +415,13 @@ def modify_generated_context(context, model, model_dict, config, manifest):
return context


def generate_operation_macro(model, config, manifest, provider):
"""This is an ugly hack to support the fact that the `docs generate`
operation ends up in here, and macros are not nodes.
def generate_execute_macro(model, config, manifest, provider):
"""Internally, macros can be executed like nodes, with some restrictions:

- they don't have have all values available that nodes do:
- 'this', 'pre_hooks', 'post_hooks', and 'sql' are missing
- 'schema' does not use any 'model' information
- they can't be configured with config() directives
"""
model_dict = model.serialize()
context = generate_base(model, model_dict, config, manifest,
Expand All @@ -447,6 +435,10 @@ def generate_model(model, config, manifest, source_config, provider):
model_dict = model.to_dict()
context = generate_base(model, model_dict, config, manifest,
source_config, provider)
# operations (hooks) don't get a 'this'
if model.resource_type != NodeType.Operation:
this = get_this_relation(context['adapter'], config, model_dict)
context['this'] = this
beckjake marked this conversation as resolved.
Show resolved Hide resolved
# overwrite schema if we have it, and hooks + sql
context.update({
'schema': model.get('schema', context['schema']),
Expand All @@ -467,7 +459,7 @@ def generate(model, config, manifest, source_config=None, provider=None):
dbt.context.runtime.generate
"""
if isinstance(model, ParsedMacro):
return generate_operation_macro(model, config, manifest, provider)
return generate_execute_macro(model, config, manifest, provider)
else:
return generate_model(model, config, manifest, source_config,
provider)
7 changes: 0 additions & 7 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,6 @@ def find_docs_by_name(self, name, package=None):
return doc
return None

def find_operation_by_name(self, name, package):
"""Find a macro in the graph by its name and package name, or None for
any package.
"""
return self._find_by_name(name, package, 'macros',
[NodeType.Operation])

def find_macro_by_name(self, name, package):
"""Find a macro in the graph by its name and package name, or None for
any package.
Expand Down
1 change: 0 additions & 1 deletion core/dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,6 @@ class ParsedNodePatch(APIObject):
'resource_type': {
'enum': [
NodeType.Macro,
NodeType.Operation,
],
},
'unique_id': {
Expand Down
3 changes: 0 additions & 3 deletions core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@
NodeType.Model,
NodeType.Test,
NodeType.Analysis,
# Note: Hooks fail if you remove this, even though it's
# also allowed in ParsedMacro, which seems wrong.
# Maybe need to move hook operations into macros?
NodeType.Operation,
NodeType.Seed,
# we need this if parse_node is going to handle archives.
Expand Down

This file was deleted.

This file was deleted.

7 changes: 1 addition & 6 deletions core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,8 @@ def get_schema_func(self):
def get_schema(_):
return self.default_schema
else:
# use the macro itself as the 'parsed node' to pass into
# parser.generate() to get a context.
macro_node = get_schema_macro.incorporate(
resource_type=NodeType.Operation
)
root_context = dbt.context.parser.generate(
macro_node, self.root_project_config,
get_schema_macro, self.root_project_config,
self.macro_manifest, self.root_project_config
)
get_schema = get_schema_macro.generator(root_context)
Expand Down
4 changes: 0 additions & 4 deletions core/dbt/parser/macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ def parse_macro_file(self, macro_file_path, macro_file_contents, root_path,
node_type = NodeType.Macro
name = macro_name.replace(dbt.utils.MACRO_PREFIX, '')

elif macro_name.startswith(dbt.utils.OPERATION_PREFIX):
node_type = NodeType.Operation
name = macro_name.replace(dbt.utils.OPERATION_PREFIX, '')

if node_type != resource_type:
continue

Expand Down
12 changes: 0 additions & 12 deletions core/dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,13 @@ def find_in_list_by_name(haystack, target_name, target_package, nodetype):


MACRO_PREFIX = 'dbt_macro__'
OPERATION_PREFIX = 'dbt_operation__'
DOCS_PREFIX = 'dbt_docs__'


def get_dbt_macro_name(name):
return '{}{}'.format(MACRO_PREFIX, name)


def get_dbt_operation_name(name):
return '{}{}'.format(OPERATION_PREFIX, name)


def get_dbt_docs_name(name):
return '{}{}'.format(DOCS_PREFIX, name)

Expand All @@ -180,13 +175,6 @@ def get_materialization_macro_name(materialization_name, adapter_type=None,
return name


def get_operation_macro_name(operation_name, with_prefix=True):
if with_prefix:
return get_dbt_operation_name(operation_name)
else:
return operation_name


def get_docs_macro_name(docs_name, with_prefix=True):
if with_prefix:
return get_dbt_docs_name(docs_name)
Expand Down
6 changes: 3 additions & 3 deletions plugins/postgres/dbt/adapters/postgres/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from dbt.logger import GLOBAL_LOGGER as logger


GET_RELATIONS_OPERATION_NAME = 'get_relations_data'
GET_RELATIONS_MACRO_NAME = 'get_relations'


class PostgresAdapter(SQLAdapter):
Expand All @@ -24,9 +24,9 @@ def date_function(cls):
def _link_cached_relations(self, manifest):
schemas = manifest.get_used_schemas()
try:
table = self.run_operation(manifest, GET_RELATIONS_OPERATION_NAME)
table = self.execute_macro(manifest, GET_RELATIONS_MACRO_NAME)
finally:
self.release_connection(GET_RELATIONS_OPERATION_NAME)
self.release_connection(GET_RELATIONS_MACRO_NAME)
table = self._relations_filter_table(table, schemas)

for (refed_schema, refed_name, dep_schema, dep_name) in table:
Expand Down
8 changes: 4 additions & 4 deletions test/unit/test_postgres_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ def test_set_zero_keepalive(self, psycopg2):
port=5432,
connect_timeout=10)

@mock.patch.object(PostgresAdapter, 'run_operation')
def test_get_catalog_various_schemas(self, mock_run):
@mock.patch.object(PostgresAdapter, 'execute_macro')
def test_get_catalog_various_schemas(self, mock_execute):
column_names = ['table_schema', 'table_name']
rows = [
('foo', 'bar'),
Expand All @@ -143,8 +143,8 @@ def test_get_catalog_various_schemas(self, mock_run):
('quux', 'bar'),
('skip', 'bar')
]
mock_run.return_value = agate.Table(rows=rows,
column_names=column_names)
mock_execute.return_value = agate.Table(rows=rows,
column_names=column_names)

mock_manifest = mock.MagicMock()
mock_manifest.get_used_schemas.return_value = {'foo', 'quux'}
Expand Down