Skip to content

ELE-890 workaround for redshift run operation regression - update alerts#880

Merged
IDoneShaveIt merged 4 commits intomasterfrom
ele-890-workaround-for-redshift-run-operation
May 17, 2023
Merged

ELE-890 workaround for redshift run operation regression - update alerts#880
IDoneShaveIt merged 4 commits intomasterfrom
ele-890-workaround-for-redshift-run-operation

Conversation

@IDoneShaveIt
Copy link
Copy Markdown
Contributor

Due to a bug with dbt-redshift 1.5.0 and above, we can't update tables using dbt run-operation.
In this PR I am replacing the use of dbt run-operation to update our alerts status, with dbt run (models).

@IDoneShaveIt IDoneShaveIt requested a review from Maayan-s May 16, 2023 15:05
@linear
Copy link
Copy Markdown

linear bot commented May 16, 2023

ELE-890 Workaround for redshift run-operation update or insert regression

Describe the problem
On dbt-redshift 1.5.0 and above we can run update / insert queries (the run is complete, but the changes are not committed - run adapter.commit is not possible as well)

@github-actions
Copy link
Copy Markdown
Contributor

👋 @IDoneShaveIt
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 this pull request.

{{ elementary.edr_cast_as_timestamp('detected_at') }} >= {{ get_alerts_time_limit() }}
{% endset %}
{% do elementary.run_query(update_sent_alerts_query) %}
{% if execute %}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wrapped the current macro with if execute

{{ elementary.edr_cast_as_timestamp('detected_at') }} >= {{ get_alerts_time_limit() }}
{% endset %}
{% do elementary.run_query(update_skipped_alerts_query) %}
{% if execute %}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wrapped the current macro with if execute

@Maayan-s Maayan-s linked an issue May 16, 2023 that may be closed by this pull request
@IDoneShaveIt IDoneShaveIt requested a review from haritamar May 17, 2023 05:17
@@ -35,9 +35,13 @@ def test_update_sent_alerts(
calls_args = mock_subprocess_run.call_args_list
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@IDoneShaveIt
We don't actually test that the status was updated. No?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will add a GitHub action for that

@IDoneShaveIt IDoneShaveIt merged commit ab5b54c into master May 17, 2023
@IDoneShaveIt IDoneShaveIt deleted the ele-890-workaround-for-redshift-run-operation branch May 17, 2023 08:13
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.

[ELE-892] Alerts sent status update fails with dbt-redshift 1.5.0

2 participants