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

add parser #292

Merged
merged 27 commits into from
Mar 2, 2017
Merged

add parser #292

merged 27 commits into from
Mar 2, 2017

Conversation

cmcarthur
Copy link
Member

No description provided.

model for model in all_models
if model.is_enabled and not model.is_empty
model for model in parsed_models
if model.get('enabled') == True and model.get('empty') == True
Copy link
Contributor

Choose a reason for hiding this comment

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

we probably want empty = False here, right?

dbt/parser.py Outdated
env = jinja2.sandbox.SandboxedEnvironment(
undefined=SilentUndefined)

env.from_string(model.get('raw_sql')).render(context)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


filepath = os.path.join(root, model.rel_filepath)
logger.info("Compiler error in {}".format(filepath))
logger.info("Compiler error in {}".format(model.get('path')))
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

for name, injected_model in injected_models.items():
# now turn a model back into the old-style model object
model = Model(
self.project,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we always want self.project here?

Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

this is looking great! couple of notes / questions but i understand that this is still in development

profile = self.project.run_environment()
compiled_model.compile(context, profile, self.existing_models)
compiled_models.append(compiled_model)
post_filter = [
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is complex enough that it warrants a for loop!

"{{ col.name }}" {% if not loop.last %},{% endif %}
{% endfor %},
{% endraw %}
{% for col in get_columns_in_table(source_schema, source_table) %}
Copy link
Contributor

Choose a reason for hiding this comment

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

beautiful

@@ -291,7 +291,7 @@ def rename(cls, profile, from_name, to_name, model_name=None):

@classmethod
def execute_model(cls, profile, model):
parts = re.split(r'-- (DBT_OPERATION .*)', model.compiled_contents)
parts = re.split(r'-- (DBT_OPERATION .*)', model.get('wrapped_sql'))
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll be able to replace this with separate nodes grouped into a transaction, right?

logger.info("Enabled models:")
for m in all_models:
logger.info(" - {}".format(".".join(m.fqn)))
for n,m in all_models.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

for m in all_models.values() :)

context['config'] = self.__model_config(model, linker)
context['this'] = This(
context['env']['schema'],
(model.get('name') if dbt.flags.NON_DESTRUCTIVE
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 ported from model.tmp_name(), right? it took me a minute to work out what was happening here... maybe we could add a comment like:

for non-destructive runs, this should return the final name of the model, because we're truncating and inserting instead dropping and re-creating.

self.project_schemas
)
elif injected_node.get('resource_type') == NodeType.Archive:
# unfortunately we do everything automagically for
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

@@ -112,9 +121,21 @@ def get_nodes_from_spec(project, graph, spec):
if select_children:
for node in selected_nodes:
child_nodes = nx.descendants(graph, node)
print('\nchild_nodes')
Copy link
Contributor

Choose a reason for hiding this comment

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

:)

dbt/runner.py Outdated

rows = cursor.fetchall()

adapter.rollback(profile)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's going on here?

dbt/runner.py Outdated
output = ("START archive table {schema}.{model_name} "
.format(**print_vars))
return output
print('hooks')
Copy link
Contributor

Choose a reason for hiding this comment

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

:)

output = "{info} table {schema}.{model_name} ".format(**print_vars)
return output
compiled_hooks = [
dbt.compilation.compile_string(hook, ctx) for hook in hooks
Copy link
Contributor

Choose a reason for hiding this comment

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

hooks should also have access to their model context, eg. this or var. Does that happen here?

Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

@cmcarthur excited to get this merged. Some comments for my edification, but otherwise lgtm

@@ -0,0 +1,45 @@
import codecs
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 great

@@ -182,7 +182,7 @@ def rename(cls, profile, from_name, to_name, model_name=None):

@classmethod
def execute_model(cls, profile, model):
parts = re.split(r'-- (DBT_OPERATION .*)', model.compiled_contents)
parts = re.split(r'-- (DBT_OPERATION .*)', model.get('wrapped_sql'))
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll break these out into separate nodes in a future PR, yeah?

Copy link
Member Author

Choose a reason for hiding this comment

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

absolutely

@cmcarthur cmcarthur merged commit 28b8067 into development Mar 2, 2017
@cmcarthur cmcarthur deleted the add-parser branch March 2, 2017 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants