Skip to content

Commit 7137614

Browse files
committed
Fixed Itamar's CR
2 parents c726d79 + 4e29bda commit 7137614

File tree

15 files changed

+380
-1710
lines changed

15 files changed

+380
-1710
lines changed

.github/workflows/test-warehouse.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ jobs:
144144
uses: actions/upload-artifact@v3
145145
with:
146146
name: report_${{ inputs.warehouse-type }}_${{ env.BRANCH_NAME }}.html
147-
path: report_${{ inputs.warehouse-type }}_${{ env.BRANCH_NAME }}.html
147+
path: ${{ env.TESTS_DIR }}/report_${{ inputs.warehouse-type }}_${{ env.BRANCH_NAME }}.html
148148

149149
- name: Write GCS keyfile
150150
env:
Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
from typing import Optional
23

34
from elementary.clients.api.api import APIClient
45
from elementary.monitor.api.invocations.schema import DbtInvocationSchema
@@ -10,42 +11,35 @@
1011
class InvocationsAPI(APIClient):
1112
def get_last_invocation(self, type: str) -> DbtInvocationSchema:
1213
if type == "test":
13-
invocation_response = self.dbt_runner.run_operation(
14-
macro_name="get_test_last_invocation",
15-
)
16-
invocation = (
17-
json.loads(invocation_response[0]) if invocation_response else None
18-
)
19-
return DbtInvocationSchema(**invocation[0])
14+
return self._get_test_last_invocation()
2015
else:
2116
raise NotImplementedError
2217

2318
def get_invocation_by_time(
2419
self, type: str, invocation_max_time: str
2520
) -> DbtInvocationSchema:
2621
if type == "test":
27-
invocation_response = self.dbt_runner.run_operation(
28-
macro_name="get_test_last_invocation",
29-
macro_args=dict(invocation_max_time=invocation_max_time),
30-
)
31-
invocation = (
32-
json.loads(invocation_response[0]) if invocation_response else None
22+
return self._get_test_last_invocation(
23+
macro_args=dict(invocation_max_time=invocation_max_time)
3324
)
34-
return DbtInvocationSchema(**invocation[0])
3525
else:
3626
raise NotImplementedError
3727

3828
def get_invocation_by_id(
3929
self, type: str, invocation_id: str
4030
) -> DbtInvocationSchema:
4131
if type == "test":
42-
invocation_response = self.dbt_runner.run_operation(
43-
macro_name="get_test_last_invocation",
44-
macro_args=dict(invocation_id=invocation_id),
32+
return self._get_test_last_invocation(
33+
macro_args=dict(invocation_id=invocation_id)
4534
)
46-
invocation = (
47-
json.loads(invocation_response[0]) if invocation_response else None
48-
)
49-
return DbtInvocationSchema(**invocation[0])
5035
else:
5136
raise NotImplementedError
37+
38+
def _get_test_last_invocation(
39+
self, macro_args: Optional[dict] = None
40+
) -> DbtInvocationSchema:
41+
invocation_response = self.dbt_runner.run_operation(
42+
macro_name="get_test_last_invocation", macro_args=macro_args
43+
)
44+
invocation = json.loads(invocation_response[0]) if invocation_response else None
45+
return DbtInvocationSchema(**invocation[0])

elementary/monitor/api/tests/tests.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
class TestsAPI(APIClient):
4040
def __init__(self, dbt_runner: DbtRunner):
4141
super().__init__(dbt_runner)
42-
self.invocatons_api = InvocationsAPI(dbt_runner)
42+
self.invocations_api = InvocationsAPI(dbt_runner)
4343

4444
@staticmethod
4545
def get_test_sub_type_unique_id(
@@ -74,17 +74,18 @@ def get_tests_metadata(
7474
def _get_invocation_from_filter(
7575
self, filter: DataMonitoringFilter
7676
) -> Optional[DbtInvocationSchema]:
77+
# If none of the following filter options exists, the invocation is empty and there is no filter.
7778
invocation = DbtInvocationSchema()
7879
if filter.invocation_id:
79-
invocation = self.invocatons_api.get_invocation_by_id(
80+
invocation = self.invocations_api.get_invocation_by_id(
8081
type="test", invocation_id=filter.invocation_id
8182
)
8283
elif filter.invocation_time:
83-
invocation = self.invocatons_api.get_invocation_by_time(
84+
invocation = self.invocations_api.get_invocation_by_time(
8485
type="test", invocation_max_time=filter.invocation_time
8586
)
8687
elif filter.last_invocation:
87-
invocation = self.invocatons_api.get_last_invocation(type="test")
88+
invocation = self.invocations_api.get_last_invocation(type="test")
8889

8990
self.set_run_cache(key=INVOCATION, value=invocation)
9091
return invocation
@@ -213,7 +214,7 @@ def get_test_results(
213214
for test_metadata in test_results_metadata:
214215
test_metadata_dict = dict(test_metadata)
215216
test_sub_type_unique_id = self.get_test_sub_type_unique_id(
216-
**dict(test_metadata_dict)
217+
**test_metadata_dict
217218
)
218219
test_sample_data = tests_sample_data.get(test_sub_type_unique_id)
219220
test_result = TestResultSchema(
@@ -242,7 +243,7 @@ def get_test_runs(
242243
for test_metadata in test_results_metadata:
243244
test_metadata_dict = dict(test_metadata)
244245
test_sub_type_unique_id = self.get_test_sub_type_unique_id(
245-
**dict(test_metadata_dict)
246+
**test_metadata_dict
246247
)
247248
test_invocations = tests_invocations.get(test_sub_type_unique_id)
248249
test_run = TestRunSchema(

elementary/monitor/cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def get_cli_properties() -> dict:
159159
)
160160
@click.option(
161161
"--deprecated-slack-webhook",
162-
"-s", # Deprecatred - will be used for --select in the future
162+
"-s", # Deprecated - will be used for --select in the future
163163
type=str,
164164
default=None,
165165
help="DEPRECATED! - A slack webhook URL for sending alerts to a specific channel.",

elementary/monitor/data_monitoring/data_monitoring_report.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ def _parse_filter(self, filter: Optional[str] = None) -> DataMonitoringFilter:
6060
data_monitoring_filter = DataMonitoringFilter()
6161
if filter:
6262
invocation_id_regex = re.compile(r"invocation_id:\w+")
63-
invocation_time_regex = re.compile(
64-
r"invocation_time:([\+-]?\d{4}(?!\d{2}\b))((-?)((0[1-9]|1[0-2])(\3([12]\d|0[1-9]|3[01]))?|W([0-4]\d|5[0-2])(-?[1-7])?|(00[1-9]|0[1-9]\d|[12]\d{2}|3([0-5]\d|6[1-6])))([T\s]((([01]\d|2[0-3])((:?)[0-5]\d)?|24\:?00))?(\17[0-5]\d)))"
65-
)
63+
invocation_time_regex = re.compile(r"invocation_time:\w+")
6664
last_invocation_regex = re.compile(r"last_invocation")
6765

6866
invocation_id_match = invocation_id_regex.search(filter)

elementary/monitor/data_monitoring/index.html

Lines changed: 211 additions & 1549 deletions
Large diffs are not rendered by default.

elementary/monitor/data_monitoring/schema.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33

44
from pydantic import BaseModel, validator
55

6+
from elementary.utils.log import get_logger
67
from elementary.utils.time import DATETIME_FORMAT, convert_local_time_to_timezone
78

9+
logger = get_logger(__name__)
10+
811

912
class DataMonitoringFilter(BaseModel):
1013
invocation_id: Optional[str] = None
@@ -14,8 +17,14 @@ class DataMonitoringFilter(BaseModel):
1417
@validator("invocation_time", pre=True)
1518
def format_invocation_time(cls, invocation_time):
1619
if invocation_time:
17-
invocation_datetime = convert_local_time_to_timezone(
18-
datetime.fromisoformat(invocation_time)
19-
)
20-
return invocation_datetime.strftime(DATETIME_FORMAT)
20+
try:
21+
invocation_datetime = convert_local_time_to_timezone(
22+
datetime.fromisoformat(invocation_time)
23+
)
24+
return invocation_datetime.strftime(DATETIME_FORMAT)
25+
except ValueError as err:
26+
logger.error(
27+
f"Failed to parse invocaton time filter: {err}\nPlease use a valid ISO 8601 format"
28+
)
29+
raise
2130
return None

elementary/monitor/dbt_project/macros/empty_alert_tables.sql renamed to elementary/monitor/dbt_project/macros/alerts/empty_alert_tables.sql

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
('status', 'string'),
1515
('full_refresh', 'boolean'),
1616
('alert_sent', 'boolean'),
17-
('original_path', 'string')
17+
('original_path', 'string'),
18+
('suppression_status', 'string'),
19+
('sent_at', 'string')
1820
]) }}
1921
{% endmacro %}
2022

@@ -40,6 +42,8 @@
4042
('owner', 'string'),
4143
('package_name', 'string'),
4244
('path', 'string'),
43-
('alert_sent', 'boolean')
45+
('alert_sent', 'boolean'),
46+
('suppression_status', 'string'),
47+
('sent_at', 'string')
4448
]) }}
4549
{% endmacro %}

elementary/monitor/dbt_project/macros/alerts/get_model_alerts.sql

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,37 +27,40 @@
2727
union all
2828
select unique_id, meta from {{ snapshots_relation }}
2929
{% endif %}
30+
),
31+
32+
extended_alerts as (
33+
select
34+
alerts_in_time_limit.alert_id,
35+
alerts_in_time_limit.unique_id,
36+
alerts_in_time_limit.detected_at,
37+
alerts_in_time_limit.database_name,
38+
alerts_in_time_limit.materialization,
39+
alerts_in_time_limit.path,
40+
alerts_in_time_limit.original_path,
41+
alerts_in_time_limit.schema_name,
42+
alerts_in_time_limit.message,
43+
alerts_in_time_limit.owners,
44+
alerts_in_time_limit.tags,
45+
alerts_in_time_limit.alias,
46+
alerts_in_time_limit.status,
47+
alerts_in_time_limit.full_refresh,
48+
{# backwards compatibility #}
49+
case
50+
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = TRUE then 'sent'
51+
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = FALSE then 'pending'
52+
else suppression_status
53+
end as suppression_status,
54+
alerts_in_time_limit.sent_at,
55+
artifacts_meta.meta as model_meta
56+
from alerts_in_time_limit
57+
left join models on alerts_in_time_limit.unique_id = models.unique_id
58+
left join artifacts_meta on alerts_in_time_limit.unique_id = artifacts_meta.unique_id
3059
)
31-
select
32-
alerts_in_time_limit.alert_id,
33-
alerts_in_time_limit.unique_id,
34-
alerts_in_time_limit.detected_at,
35-
alerts_in_time_limit.database_name,
36-
alerts_in_time_limit.materialization,
37-
alerts_in_time_limit.path,
38-
alerts_in_time_limit.original_path,
39-
alerts_in_time_limit.schema_name,
40-
alerts_in_time_limit.message,
41-
alerts_in_time_limit.owners,
42-
alerts_in_time_limit.tags,
43-
alerts_in_time_limit.alias,
44-
alerts_in_time_limit.status,
45-
alerts_in_time_limit.full_refresh,
46-
{# backwards compatibility #}
47-
case
48-
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = TRUE then 'sent'
49-
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = FALSE then 'pending'
50-
else suppression_status
51-
end as suppression_status,
52-
case
53-
when alerts_in_time_limit.sent_at is NULL then '1970-01-01 00:00:00'
54-
else alerts_in_time_limit.sent_at
55-
end as sent_at,
56-
artifacts_meta.meta as model_meta
57-
from alerts_in_time_limit
58-
left join models on alerts_in_time_limit.unique_id = models.unique_id
59-
left join artifacts_meta on alerts_in_time_limit.unique_id = artifacts_meta.unique_id
60-
having suppression_status = 'pending'
60+
61+
select *
62+
from extended_alerts
63+
where suppression_status = 'pending'
6164
{% endset %}
6265

6366
{% set alerts_agate = run_query(select_pending_alerts_query) %}
@@ -99,10 +102,7 @@
99102
when suppression_status is NULL and alert_sent = FALSE then 'pending'
100103
else suppression_status
101104
end as suppression_status,
102-
case
103-
when sent_at is NULL then '1970-01-01 00:00:00'
104-
else sent_at
105-
end as sent_at
105+
sent_at
106106
from {{ ref('alerts_models') }}
107107
where {{ elementary.cast_as_timestamp('detected_at') }} >= {{ get_alerts_time_limit(days_back) }}
108108
)

elementary/monitor/dbt_project/macros/alerts/get_source_freshness_alerts.sql

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,43 +18,45 @@
1818
select unique_id, meta from models
1919
union all
2020
select unique_id, meta from sources
21+
),
22+
23+
extended_alerts as (
24+
select
25+
alerts_in_time_limit.alert_id,
26+
alerts_in_time_limit.max_loaded_at,
27+
alerts_in_time_limit.snapshotted_at,
28+
alerts_in_time_limit.detected_at,
29+
alerts_in_time_limit.max_loaded_at_time_ago_in_s,
30+
alerts_in_time_limit.status,
31+
alerts_in_time_limit.error,
32+
alerts_in_time_limit.unique_id,
33+
alerts_in_time_limit.database_name,
34+
alerts_in_time_limit.schema_name,
35+
alerts_in_time_limit.source_name,
36+
alerts_in_time_limit.identifier,
37+
alerts_in_time_limit.freshness_error_after,
38+
alerts_in_time_limit.freshness_warn_after,
39+
alerts_in_time_limit.freshness_filter,
40+
alerts_in_time_limit.tags,
41+
alerts_in_time_limit.meta,
42+
alerts_in_time_limit.owner,
43+
alerts_in_time_limit.package_name,
44+
alerts_in_time_limit.path,
45+
{# backwards compatibility #}
46+
case
47+
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = TRUE then 'sent'
48+
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = FALSE then 'pending'
49+
else suppression_status
50+
end as suppression_status,
51+
alerts_in_time_limit.sent_at,
52+
artifacts_meta.meta as model_meta
53+
from alerts_in_time_limit
54+
left join artifacts_meta on alerts_in_time_limit.unique_id = artifacts_meta.unique_id
2155
)
2256

23-
select
24-
alerts_in_time_limit.alert_id,
25-
alerts_in_time_limit.max_loaded_at,
26-
alerts_in_time_limit.snapshotted_at,
27-
alerts_in_time_limit.detected_at,
28-
alerts_in_time_limit.max_loaded_at_time_ago_in_s,
29-
alerts_in_time_limit.status,
30-
alerts_in_time_limit.error,
31-
alerts_in_time_limit.unique_id,
32-
alerts_in_time_limit.database_name,
33-
alerts_in_time_limit.schema_name,
34-
alerts_in_time_limit.source_name,
35-
alerts_in_time_limit.identifier,
36-
alerts_in_time_limit.freshness_error_after,
37-
alerts_in_time_limit.freshness_warn_after,
38-
alerts_in_time_limit.freshness_filter,
39-
alerts_in_time_limit.tags,
40-
alerts_in_time_limit.meta,
41-
alerts_in_time_limit.owner,
42-
alerts_in_time_limit.package_name,
43-
alerts_in_time_limit.path,
44-
{# backwards compatibility #}
45-
case
46-
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = TRUE then 'sent'
47-
when alerts_in_time_limit.suppression_status is NULL and alerts_in_time_limit.alert_sent = FALSE then 'pending'
48-
else suppression_status
49-
end as suppression_status,
50-
case
51-
when alerts_in_time_limit.sent_at is NULL then '1970-01-01 00:00:00'
52-
else alerts_in_time_limit.sent_at
53-
end as sent_at,
54-
artifacts_meta.meta as model_meta
55-
from alerts_in_time_limit
56-
left join artifacts_meta on alerts_in_time_limit.unique_id = artifacts_meta.unique_id
57-
having suppression_status = 'pending'
57+
select *
58+
from extended_alerts
59+
where suppression_status = 'pending'
5860
{% endset %}
5961

6062
{% set alerts_agate = run_query(select_pending_alerts_query) %}
@@ -100,10 +102,7 @@
100102
when suppression_status is NULL and alert_sent = FALSE then 'pending'
101103
else suppression_status
102104
end as suppression_status,
103-
case
104-
when sent_at is NULL then '1970-01-01 00:00:00'
105-
else sent_at
106-
end as sent_at
105+
sent_at
107106
from {{ ref('alerts_source_freshness') }}
108107
where {{ elementary.cast_as_timestamp('detected_at') }} >= {{ get_alerts_time_limit(days_back) }}
109108
)

0 commit comments

Comments
 (0)