Fix missing value in upload model executions - add else clause to for if loops

[kenpokiwi]

Overview

See details in #385

Update type - breaking / non-breaking

• Minor bug fix
• Documentation improvements
• Quality of Life improvements
• New features (non-breaking change)
• New features (breaking change)
• Other (non-breaking change)
• Other (breaking change)

What does this solve?

Resolves #385

Outstanding questions

What databases have you tested with?

• Snowflake
• Google BigQuery
• Databricks
• Spark
• Postgres
• N/A

[glsdown]

I think originally this loop.length section was designed for the use-case you are fixing here, but doesn't make sense (it wouldn't run if the loop was empty). That means it can be removed I believe. Looking at the logic here, it also looks like it could be hugely simplified using Jinja filtering:

                {% set compile_started_at = (model.timing | selectattr("name", "eq", "compile") | first | default({})).get("started_at") %}
                {% if compile_started_at %}'{{ compile_started_at }}'{% else %}null{% endif %}, {# compile_started_at #}
                {% set query_completed_at = (model.timing | selectattr("name", "eq", "execute") | first | default({})).get("completed_at") %}
                {% if query_completed_at %}'{{ query_completed_at }}'{% else %}null{% endif %}, {# query_completed_at #}

I think this would aid readability and consistency. What do you think?

[kenpokiwi]

Yes I think that is much more elegant :)

[glsdown]

Thanks for looking at this. For consistency, please could you also add the same logic into the other two macros in here for BQ and Snowflalke? From lines 88 and 164. Thanks!

[kenpokiwi]

Thanks for the feedback. Update done.

[glsdown]

Thanks for all your work on this!