-
Notifications
You must be signed in to change notification settings - Fork 8
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
possibility to use templated properties to apply by default on file update #38
base: main
Are you sure you want to change the base?
Changes from 3 commits
e79f21b
9e8d9c5
25ef8c5
5c502e3
d3db869
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -715,6 +715,7 @@ def _create_property_file( | |
property_path, | ||
resource_dict, | ||
columns, | ||
ctx.config['template_yml'] | ||
) | ||
_utils.write_yaml( | ||
property_path, | ||
|
@@ -786,7 +787,12 @@ def _get_columns(ctx, resource_location, resource_dict, **kwargs): | |
return columns | ||
|
||
|
||
def _structure_property_file_dict(location, resource_dict, columns_list): | ||
def _structure_property_file_dict( | ||
location, | ||
resource_dict, | ||
columns_list, | ||
template_yml | ||
): | ||
""" | ||
Structure a dictionary that will be used to create a property file | ||
|
||
|
@@ -813,6 +819,12 @@ def _structure_property_file_dict(location, resource_dict, columns_list): | |
property_file_dict = _get_property_header(resource_name, resource_type) | ||
# Get the sub-dictionaries of each existing column | ||
resource_type_plural = _SUPPORTED_RESOURCE_TYPES[resource_type] | ||
# If dbt_invoke_template.yml exists, adds templated properties to header | ||
# when not already present | ||
if template_yml and resource_type in template_yml: | ||
_apply_template( | ||
property_file_dict[resource_type_plural][0], template_yml[resource_type] | ||
) | ||
existing_columns_dict = { | ||
item['name']: item | ||
for item in property_file_dict[resource_type_plural][0]['columns'] | ||
|
@@ -825,12 +837,33 @@ def _structure_property_file_dict(location, resource_dict, columns_list): | |
column_dict = existing_columns_dict.get( | ||
column, _get_property_column(column) | ||
) | ||
# If dbt_invoke_template.yml exists, adds templated properties to column | ||
# when not already present | ||
if template_yml and 'columns' in template_yml.get(resource_type): | ||
david-whimsical marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_apply_template( | ||
column_dict, template_yml[resource_type]['columns'] | ||
) | ||
property_file_dict[resource_type_plural][0]['columns'].append( | ||
column_dict | ||
) | ||
return property_file_dict | ||
|
||
|
||
def _apply_template(property_dict, template): | ||
""" | ||
Updates a dictionary with template's yml default properties | ||
if said dictionary does not already contain the properties. | ||
|
||
:param property_dict: the dictionary to be updated | ||
:param template: a dictionary representing the templated properties | ||
to be applied on property_dict | ||
:return: None | ||
""" | ||
for key in template: | ||
if key not in property_dict: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would keep the value already in the file – else I think it could lead to frustrating behaviors, where dbt-invoke overwrites things. In my opinion, The template should only add new lines, and not update any existing ones, if that makes sense! |
||
property_dict[key] = template[key] | ||
|
||
|
||
def _get_property_header(resource, resource_type, properties=None): | ||
""" | ||
Create a dictionary representing resources properties | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be nitpicking a bit here, but I think we should change
model
,seed
,snapshot
, andanalysis
to their plural versions;models
,seeds
,snapshots
, andanalyses
so thatdbt_invoke_template.yml
matches the convention used in property files. Thoughts?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you! I think it's better to align to the used convention, I'll update
properties.py
to use plural versions instead.