From 869bdc454d1ab500548ab783cd0216f5dc245511 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Tue, 5 May 2020 08:52:49 -0600 Subject: [PATCH 1/2] Disabling tracking no longer resets the invocation ID - re-use the same active user - add disable_tracking method that does not reset invocation ID --- CHANGELOG.md | 7 +++- core/dbt/config/runtime.py | 2 +- core/dbt/tracking.py | 16 ++++++++- test/unit/test_tracking.py | 71 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 test/unit/test_tracking.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4079be60a26..99572f3da45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ -## dbt next (release TBD) +## dbt 0.17.0 (Release TBD) + +### Fixes +- When tracking is disabled due to errors, do not reset the invocation ID ([#2398](https://github.com/fishtown-analytics/dbt/issues/2398), [#2400](https://github.com/fishtown-analytics/dbt/pull/2400)) + +## dbt 0.17.0b1 (May 5, 2020) ### Breaking changes - Added a new dbt_project.yml version format. This emits a deprecation warning currently, but support for the existing version will be removed in a future dbt version ([#2300](https://github.com/fishtown-analytics/dbt/issues/2300), [#2312](https://github.com/fishtown-analytics/dbt/pull/2312)) diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index c15c1cbfe40..698c461a1eb 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -522,7 +522,7 @@ def _get_rendered_profile( # return the poisoned form profile = UnsetProfile() # disable anonymous usage statistics - tracking.do_not_track() + tracking.disable_tracking() return profile @classmethod diff --git a/core/dbt/tracking.py b/core/dbt/tracking.py index 612e6e40dd7..f42c22d404a 100644 --- a/core/dbt/tracking.py +++ b/core/dbt/tracking.py @@ -44,7 +44,7 @@ def handle_failure(num_ok, unsent): # num_ok will always be 0, unsent will always be 1 entry long, because # the buffer is length 1, so not much to talk about logger.warning('Error sending message, disabling tracking') - do_not_track() + disable_tracking() def _log_request(self, request, payload): sp_logger.info(f"Sending {request} request to {self.endpoint}...") @@ -112,6 +112,12 @@ def initialize(self): subject.set_user_id(self.id) tracker.set_subject(subject) + def disable_tracking(self): + self.do_not_track = True + self.id = None + self.cookie_dir = None + tracker.set_subject(None) + def set_cookie(self): # If the user points dbt to a profile directory which exists AND # contains a profiles.yml file, then we can set a cookie. If the @@ -364,6 +370,14 @@ def flush(): tracker.flush() +def disable_tracking(): + global active_user + if active_user is not None: + active_user.disable_tracking() + else: + active_user = User(None) + + def do_not_track(): global active_user active_user = User(None) diff --git a/test/unit/test_tracking.py b/test/unit/test_tracking.py new file mode 100644 index 00000000000..16692ff7e8e --- /dev/null +++ b/test/unit/test_tracking.py @@ -0,0 +1,71 @@ +import dbt.tracking +import datetime +import shutil +import tempfile +import unittest + + +class TestTracking(unittest.TestCase): + def setUp(self): + dbt.tracking.active_user = None + self.tempdir = tempfile.mkdtemp() + + def tearDown(self): + dbt.tracking.active_user = None + shutil.rmtree(self.tempdir) + + def test_tracking_initial(self): + assert dbt.tracking.active_user is None + dbt.tracking.initialize_tracking(self.tempdir) + assert isinstance(dbt.tracking.active_user, dbt.tracking.User) + + invocation_id = dbt.tracking.active_user.invocation_id + run_started_at = dbt.tracking.active_user.run_started_at + + assert dbt.tracking.active_user.do_not_track is False + assert isinstance(dbt.tracking.active_user.id, str) + assert isinstance(invocation_id, str) + assert isinstance(run_started_at, datetime.datetime) + + dbt.tracking.disable_tracking() + assert isinstance(dbt.tracking.active_user, dbt.tracking.User) + + assert dbt.tracking.active_user.do_not_track is True + assert dbt.tracking.active_user.id is None + assert dbt.tracking.active_user.invocation_id == invocation_id + assert dbt.tracking.active_user.run_started_at == run_started_at + + # this should generate a whole new user object -> new invocation_id/run_started_at + dbt.tracking.do_not_track() + assert isinstance(dbt.tracking.active_user, dbt.tracking.User) + + assert dbt.tracking.active_user.do_not_track is True + assert dbt.tracking.active_user.id is None + assert isinstance(dbt.tracking.active_user.invocation_id, str) + assert isinstance(dbt.tracking.active_user.run_started_at, datetime.datetime) + assert dbt.tracking.active_user.invocation_id != invocation_id + assert dbt.tracking.active_user.run_started_at != run_started_at + + def test_tracking_never_ok(self): + assert dbt.tracking.active_user is None + + # this should generate a whole new user object -> new invocation_id/run_started_at + dbt.tracking.do_not_track() + assert isinstance(dbt.tracking.active_user, dbt.tracking.User) + + assert dbt.tracking.active_user.do_not_track is True + assert dbt.tracking.active_user.id is None + assert isinstance(dbt.tracking.active_user.invocation_id, str) + assert isinstance(dbt.tracking.active_user.run_started_at, datetime.datetime) + + def test_disable_never_enabled(self): + assert dbt.tracking.active_user is None + + # this should generate a whole new user object -> new invocation_id/run_started_at + dbt.tracking.disable_tracking() + assert isinstance(dbt.tracking.active_user, dbt.tracking.User) + + assert dbt.tracking.active_user.do_not_track is True + assert dbt.tracking.active_user.id is None + assert isinstance(dbt.tracking.active_user.invocation_id, str) + assert isinstance(dbt.tracking.active_user.run_started_at, datetime.datetime) From 18cfe81e004f550b562b3df9c59cc4fef94ec9be Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Tue, 5 May 2020 09:41:46 -0600 Subject: [PATCH 2/2] use is not instead of != on time comparison check --- test/unit/test_tracking.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/test_tracking.py b/test/unit/test_tracking.py index 16692ff7e8e..43a805b8745 100644 --- a/test/unit/test_tracking.py +++ b/test/unit/test_tracking.py @@ -44,7 +44,8 @@ def test_tracking_initial(self): assert isinstance(dbt.tracking.active_user.invocation_id, str) assert isinstance(dbt.tracking.active_user.run_started_at, datetime.datetime) assert dbt.tracking.active_user.invocation_id != invocation_id - assert dbt.tracking.active_user.run_started_at != run_started_at + # if you use `!=`, you might hit a race condition (especially on windows) + assert dbt.tracking.active_user.run_started_at is not run_started_at def test_tracking_never_ok(self): assert dbt.tracking.active_user is None