Ele 1684 create an unlinked tables macro #523
Conversation
ELE-1684 create an "unlinked tables" macro
Definition of done:
|
|
👋 @ofek1weiss |
| {% for schema in schemas %} | ||
| {% set db_name, schema_name = schema.split('.') %} | ||
| {% set schema_relation = api.Relation.create(db_name, schema_name).without_identifier() %} | ||
| {% set relations = list_relations_without_caching(schema_relation) %} |
| @@ -0,0 +1,59 @@ | |||
| {% macro get_tables_without_models(deprecated_models_path=none) %} | |||
There was a problem hiding this comment.
From this macro you could infer that it's somewhat related to sourced, snapshots, seeds, etc.
Maybe call it something in the area of leftover or dangling?
Not super critical I suppose.
There was a problem hiding this comment.
i like dangling, on it
| {% set schema_relation = api.Relation.create(db_name, schema_name).without_identifier() %} | ||
| {% set relations = dbt.list_relations_without_caching(schema_relation) %} | ||
| {# list_relations_without_caching can return either a list of Relation objects or an agate depending on the adapter #} | ||
| {% if relations.append is defined %} |
There was a problem hiding this comment.
this is pretty hacky, but there isnt an is list test in jinja, only an is iterable, and an agate is also iterable.
i tried to find a less hacky solution but this is the best i found
| {% if relations.append is defined %} | ||
| {# relations is a list of Relation objects #} | ||
| {% for relation in relations %} | ||
| {% do tables.append(schema ~ "." ~ relation.identifier) %} |
There was a problem hiding this comment.
I usually prefer using api.Relation.create as it takes care of stringifying the relation in the safest way (quoting, etc.).
| {% endmacro %} | ||
|
|
||
|
|
||
| {% macro get_db_tables_in_schemas(schemas) %} |
There was a problem hiding this comment.
What do you think about calling it get_tables_in_schemas?
The db confused me a bit 😅
There was a problem hiding this comment.
the thought was to separate model_tables - tables defined by models, and db_tables - tables that exist in the db, but looks like its more confusing that way
| {% if not deprecated_models_path %} | ||
| {% do relevant_nodes.append(model_node) %} | ||
| {% endif %} | ||
| {% if deprecated_models_path and not model_node.original_file_path.startswith(deprecated_models_path) %} |
There was a problem hiding this comment.
| {% if deprecated_models_path and not model_node.original_file_path.startswith(deprecated_models_path) %} | |
| {% elif not model_node.original_file_path.startswith(deprecated_models_path) %} |
| {% for model_node in relevant_nodes %} | ||
| {% set model_schema = api.Relation.create(model_node.database, model_node.schema).without_identifier() %} | ||
| {% set model_table = api.Relation.create(model_node.database, model_node.schema, model_node.name) %} | ||
| {% if model_schema not in model_schemas %} |
There was a problem hiding this comment.
You can use a set() if you want but not critical.
| {% endfor %} | ||
| {% for model_node in relevant_nodes %} | ||
| {% set model_schema = api.Relation.create(model_node.database, model_node.schema).without_identifier() %} | ||
| {% set model_table = api.Relation.create(model_node.database, model_node.schema, model_node.name) %} |
There was a problem hiding this comment.
You need to use alias, not name.
| {% for model_node in graph.nodes.values() | selectattr('resource_type', '==', 'model') %} | ||
| {% if not deprecated_models_path %} | ||
| {% do relevant_nodes.append(model_node) %} | ||
| {% elif not model_node.original_file_path.startswith(deprecated_models_path) %} |
There was a problem hiding this comment.
Can you just add this as an or above?
{% if not deprecated_models_path or not model_node.original_file_path.startswith(deprecated_models_path) %}| {% endmacro %} | ||
|
|
||
|
|
||
| {% macro get_tables_in_schemas(schemas) %} |
There was a problem hiding this comment.
Sorry for the nit-picking, but maybe call this get_all_tables_in_schemas in order to clarify that this one includes tables that are not in dbt compared to the macro below?
No description provided.