From aa2333e23f67566a371d31f62ae84bff4975fb2b Mon Sep 17 00:00:00 2001 From: "d.kaneshiro" Date: Fri, 19 May 2023 12:09:52 +0900 Subject: [PATCH 1/7] UnifiedToUTC --- .../simple_snapshot/test_hard_delete_snapshot.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py index edb7324f884..f0eb55d1f74 100644 --- a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py +++ b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py @@ -1,9 +1,5 @@ import os from datetime import datetime -from datetime import timedelta -from datetime import timezone -import time - import pytz import pytest from dbt.tests.util import run_dbt, check_relations_equal @@ -18,14 +14,6 @@ # These tests uses the same seed data, containing 20 records of which we hard delete the last 10. # These deleted records set the dbt_valid_to to time the snapshot was ran. -# Using replace on a timestamp won't account for hour differences unless given the local timezone. -# We can force python as utc but not postgres fields which need to be handled as local timestamps. -def currenttz(): - if time.daylight: - return timezone(timedelta(seconds=-time.altzone), time.tzname[1]) - else: - return timezone(timedelta(seconds=-time.timezone), time.tzname[0]) - def datetime_snapshot(): NUM_SNAPSHOT_MODELS = 1 @@ -94,7 +82,7 @@ def test_snapshot_hard_delete(project): for result in snapshotted[10:]: # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[-1], datetime) - assert result[-1].replace(tzinfo=currenttz()) >= invalidated_snapshot_datetime + assert result[-1].replace(tzinfo=pytz.UTC) >= invalidated_snapshot_datetime # revive records # Timestamp must have microseconds for tests below to be meaningful @@ -133,7 +121,7 @@ def test_snapshot_hard_delete(project): for result in invalidated_records: # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[1], datetime) - assert result[1].replace(tzinfo=currenttz()) >= invalidated_snapshot_datetime + assert result[1].replace(tzinfo=pytz.UTC) >= invalidated_snapshot_datetime # records which were revived (id = 10, 11) # dbt_valid_to is null From 16479e293ac3672334f0518e57df8047e1ac1100 Mon Sep 17 00:00:00 2001 From: "d.kaneshiro" Date: Wed, 24 May 2023 16:17:45 +0900 Subject: [PATCH 2/7] Check proximity of dbt_valid_to and deleted time --- .../test_hard_delete_snapshot.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py index f0eb55d1f74..0756759fa18 100644 --- a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py +++ b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py @@ -1,5 +1,5 @@ import os -from datetime import datetime +from datetime import datetime, timedelta import pytz import pytest from dbt.tests.util import run_dbt, check_relations_equal @@ -82,7 +82,13 @@ def test_snapshot_hard_delete(project): for result in snapshotted[10:]: # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[-1], datetime) - assert result[-1].replace(tzinfo=pytz.UTC) >= invalidated_snapshot_datetime + # Plenty of wiggle room if clocks aren't perfectly sync'd, etc + tolerance = timedelta(minutes=1) + assert ( + result[-1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) + ) and ( + result[-1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) + ), f"SQL timestamp {result[-1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {result[-1].replace(tzinfo=pytz.UTC).isoformat()}" # revive records # Timestamp must have microseconds for tests below to be meaningful @@ -121,7 +127,13 @@ def test_snapshot_hard_delete(project): for result in invalidated_records: # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[1], datetime) - assert result[1].replace(tzinfo=pytz.UTC) >= invalidated_snapshot_datetime + # Plenty of wiggle room if clocks aren't perfectly sync'd, etc + tolerance = timedelta(minutes=1) + assert ( + result[1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) + ) and ( + result[1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) + ), f"SQL timestamp {result[1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {result[1].replace(tzinfo=pytz.UTC).isoformat()}" # records which were revived (id = 10, 11) # dbt_valid_to is null From c7361976f9b0dcab63059000759a89728d64d9ab Mon Sep 17 00:00:00 2001 From: "d.kaneshiro" Date: Thu, 25 May 2023 08:57:26 +0900 Subject: [PATCH 3/7] update the message to print if the assertion fails --- tests/functional/simple_snapshot/test_hard_delete_snapshot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py index 0756759fa18..a85fef51d85 100644 --- a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py +++ b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py @@ -88,7 +88,7 @@ def test_snapshot_hard_delete(project): result[-1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) ) and ( result[-1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) - ), f"SQL timestamp {result[-1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {result[-1].replace(tzinfo=pytz.UTC).isoformat()}" + ), f"SQL timestamp {result[-1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" # revive records # Timestamp must have microseconds for tests below to be meaningful @@ -133,7 +133,7 @@ def test_snapshot_hard_delete(project): result[1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) ) and ( result[1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) - ), f"SQL timestamp {result[1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {result[1].replace(tzinfo=pytz.UTC).isoformat()}" + ), f"SQL timestamp {result[1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" # records which were revived (id = 10, 11) # dbt_valid_to is null From 5fd7b910518b99aacc3f594738430bef4a0b83b6 Mon Sep 17 00:00:00 2001 From: "d.kaneshiro" Date: Thu, 25 May 2023 09:21:59 +0900 Subject: [PATCH 4/7] add CHANGELOG entries --- .changes/unreleased/Fixes-20230525-091955.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20230525-091955.yaml diff --git a/.changes/unreleased/Fixes-20230525-091955.yaml b/.changes/unreleased/Fixes-20230525-091955.yaml new file mode 100644 index 00000000000..a2e4daf39f9 --- /dev/null +++ b/.changes/unreleased/Fixes-20230525-091955.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Unified to UTC +time: 2023-05-25T09:19:55.865281+09:00 +custom: + Author: d-kaneshiro + Issue: "7664" From b134ba84744da5a1fd6382681f23fced492d97ce Mon Sep 17 00:00:00 2001 From: "d.kaneshiro" Date: Thu, 25 May 2023 10:33:18 +0900 Subject: [PATCH 5/7] test only if naive --- .../test_hard_delete_snapshot.py | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py index a85fef51d85..15d9b284229 100644 --- a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py +++ b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py @@ -3,6 +3,7 @@ import pytz import pytest from dbt.tests.util import run_dbt, check_relations_equal +from dbt.tests.adapter.utils.test_current_timestamp import is_naive from tests.functional.simple_snapshot.fixtures import ( models__schema_yml, models__ref_snapshot_sql, @@ -83,12 +84,14 @@ def test_snapshot_hard_delete(project): # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[-1], datetime) # Plenty of wiggle room if clocks aren't perfectly sync'd, etc - tolerance = timedelta(minutes=1) - assert ( - result[-1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) - ) and ( - result[-1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) - ), f"SQL timestamp {result[-1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" + # naive only(Because in the case of aware, it is not possible to compare the time of DB and Python) + if is_naive(result[-1]): + tolerance = timedelta(minutes=1) + assert ( + result[-1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) + ) and ( + result[-1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) + ), f"SQL timestamp {result[-1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" # revive records # Timestamp must have microseconds for tests below to be meaningful @@ -128,12 +131,14 @@ def test_snapshot_hard_delete(project): # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[1], datetime) # Plenty of wiggle room if clocks aren't perfectly sync'd, etc - tolerance = timedelta(minutes=1) - assert ( - result[1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) - ) and ( - result[1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) - ), f"SQL timestamp {result[1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" + # naive only(Because in the case of aware, it is not possible to compare the time of DB and Python) + if is_naive(result[1]): + tolerance = timedelta(minutes=1) + assert ( + result[1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) + ) and ( + result[1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) + ), f"SQL timestamp {result[1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" # records which were revived (id = 10, 11) # dbt_valid_to is null From d0a3071955353fd4c8894e81bf3e5afad2939189 Mon Sep 17 00:00:00 2001 From: "d.kaneshiro" Date: Thu, 25 May 2023 14:26:40 +0900 Subject: [PATCH 6/7] Added comments about naive and aware --- .../functional/simple_snapshot/test_hard_delete_snapshot.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py index 15d9b284229..90005a26acf 100644 --- a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py +++ b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py @@ -84,7 +84,8 @@ def test_snapshot_hard_delete(project): # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[-1], datetime) # Plenty of wiggle room if clocks aren't perfectly sync'd, etc - # naive only(Because in the case of aware, it is not possible to compare the time of DB and Python) + # There are two types of datetime objects: naive and aware. In this case, we are testing only with naive objects + # because in the case of aware, it is not possible to compare the time between DB and Python if is_naive(result[-1]): tolerance = timedelta(minutes=1) assert ( @@ -131,7 +132,8 @@ def test_snapshot_hard_delete(project): # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[1], datetime) # Plenty of wiggle room if clocks aren't perfectly sync'd, etc - # naive only(Because in the case of aware, it is not possible to compare the time of DB and Python) + # There are two types of datetime objects: naive and aware. In this case, we are testing only with naive objects + # because in the case of aware, it is not possible to compare the time between DB and Python if is_naive(result[1]): tolerance = timedelta(minutes=1) assert ( From fc94492422f93b13f1a489bf07ae9c52a73e3246 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Thu, 29 Jun 2023 07:21:29 -0600 Subject: [PATCH 7/7] Generalize comparison of datetimes that are "close enough" --- .../test_hard_delete_snapshot.py | 63 ++++++++++++------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py index 90005a26acf..4b4b9e281a6 100644 --- a/tests/functional/simple_snapshot/test_hard_delete_snapshot.py +++ b/tests/functional/simple_snapshot/test_hard_delete_snapshot.py @@ -3,7 +3,7 @@ import pytz import pytest from dbt.tests.util import run_dbt, check_relations_equal -from dbt.tests.adapter.utils.test_current_timestamp import is_naive +from dbt.tests.adapter.utils.test_current_timestamp import is_aware from tests.functional.simple_snapshot.fixtures import ( models__schema_yml, models__ref_snapshot_sql, @@ -16,6 +16,31 @@ # These deleted records set the dbt_valid_to to time the snapshot was ran. +def convert_to_aware(d: datetime) -> datetime: + # There are two types of datetime objects in Python: naive and aware + # Assume any dbt snapshot timestamp that is naive is meant to represent UTC + if d is None: + return d + elif is_aware(d): + return d + else: + return d.replace(tzinfo=pytz.UTC) + + +def is_close_datetime( + dt1: datetime, dt2: datetime, atol: timedelta = timedelta(microseconds=1) +) -> bool: + # Similar to pytest.approx, math.isclose, and numpy.isclose + # Use an absolute tolerance to compare datetimes that may not be perfectly equal. + # Two None values will compare as equal. + if dt1 is None and dt2 is None: + return True + elif dt1 is not None and dt2 is not None: + return (dt1 > (dt2 - atol)) and (dt1 < (dt2 + atol)) + else: + return False + + def datetime_snapshot(): NUM_SNAPSHOT_MODELS = 1 begin_snapshot_datetime = datetime.now(pytz.UTC) @@ -83,19 +108,16 @@ def test_snapshot_hard_delete(project): for result in snapshotted[10:]: # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[-1], datetime) + dbt_valid_to = convert_to_aware(result[-1]) + # Plenty of wiggle room if clocks aren't perfectly sync'd, etc - # There are two types of datetime objects: naive and aware. In this case, we are testing only with naive objects - # because in the case of aware, it is not possible to compare the time between DB and Python - if is_naive(result[-1]): - tolerance = timedelta(minutes=1) - assert ( - result[-1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) - ) and ( - result[-1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) - ), f"SQL timestamp {result[-1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" + assert is_close_datetime( + dbt_valid_to, invalidated_snapshot_datetime, timedelta(minutes=1) + ), f"SQL timestamp {dbt_valid_to.isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" # revive records # Timestamp must have microseconds for tests below to be meaningful + # Assume `updated_at` is TIMESTAMP WITHOUT TIME ZONE that implicitly represents UTC revival_timestamp = datetime.now(pytz.UTC).strftime("%Y-%m-%d %H:%M:%S.%f") project.run_sql( """ @@ -131,16 +153,12 @@ def test_snapshot_hard_delete(project): for result in invalidated_records: # result is a tuple, the dbt_valid_to column is the latest assert isinstance(result[1], datetime) + dbt_valid_to = convert_to_aware(result[1]) + # Plenty of wiggle room if clocks aren't perfectly sync'd, etc - # There are two types of datetime objects: naive and aware. In this case, we are testing only with naive objects - # because in the case of aware, it is not possible to compare the time between DB and Python - if is_naive(result[1]): - tolerance = timedelta(minutes=1) - assert ( - result[1].replace(tzinfo=pytz.UTC) > (invalidated_snapshot_datetime - tolerance) - ) and ( - result[1].replace(tzinfo=pytz.UTC) < (invalidated_snapshot_datetime + tolerance) - ), f"SQL timestamp {result[1].replace(tzinfo=pytz.UTC).isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" + assert is_close_datetime( + dbt_valid_to, invalidated_snapshot_datetime, timedelta(minutes=1) + ), f"SQL timestamp {dbt_valid_to.isoformat()} is not close enough to Python UTC {invalidated_snapshot_datetime.isoformat()}" # records which were revived (id = 10, 11) # dbt_valid_to is null @@ -165,5 +183,8 @@ def test_snapshot_hard_delete(project): # dbt_valid_from is the same as the 'updated_at' added in the revived_rows # dbt_valid_to is null assert isinstance(result[1], datetime) - assert result[1].replace(tzinfo=pytz.UTC) <= revived_snapshot_datetime - assert result[2] is None + dbt_valid_from = convert_to_aware(result[1]) + dbt_valid_to = result[2] + + assert dbt_valid_from <= revived_snapshot_datetime + assert dbt_valid_to is None