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

Ele 3034 additional package bugfixes #707

Merged
merged 4 commits into from
May 8, 2024

Conversation

haritamar
Copy link
Collaborator

No description provided.

Copy link

linear bot commented May 8, 2024

Copy link
Contributor

github-actions bot commented May 8, 2024

👋 @haritamar
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in the elementary repository.

@@ -19,6 +19,11 @@ def test_artifacts_caching(dbt_project: DbtProject):
second_row = read_model_artifact_row(dbt_project)
assert first_row == second_row, "Artifacts are not cached at the on-run-end."


def test_artifacts_updating(dbt_project: DbtProject):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Separated the original test to two, with the second test also running for dbt 1.3.0

@@ -1,7 +1,7 @@
{% macro count_true(column_name) -%}
coalesce(sum(case when cast({{ column_name }} as {{ elementary.edr_type_bool() }}) is true then 1 else 0 end), 0)
coalesce(sum(case when cast({{ column_name }} as {{ elementary.edr_type_bool() }}) = true then 1 else 0 end), 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The "is true" syntax seems to work only for BQ, where = seems to always work and I think is good enough

@haritamar haritamar force-pushed the ele-3034-additional-package-bugfixes branch from e084062 to 0b0ec80 Compare May 8, 2024 13:48
@@ -62,7 +62,7 @@
{% do return(relations) %}
{% endmaterialization %}

{% materialization incremental, adapter="athena", supported_languages=["sql"] %}
{% materialization incremental, adapter="athena", supported_languages=["sql", "python"] %}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Athena added support for python models and because of us they don't work, so added here to the supported languages.
Added to trino as well because there's actually no harm in doing so (we did the same for redshift even though it doesn't yet support Python models)

Copy link
Contributor

@IDoneShaveIt IDoneShaveIt left a comment

Choose a reason for hiding this comment

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

LGTM

@haritamar haritamar merged commit 449da8f into master May 8, 2024
12 checks passed
@haritamar haritamar deleted the ele-3034-additional-package-bugfixes branch May 8, 2024 15:17
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