From b2ecc8ad913a7792df0c978a08b21ea1ef7d64f3 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Mon, 2 Dec 2024 15:23:37 +0000 Subject: [PATCH 1/4] remove tracking --- spectacles/cli.py | 22 ------- spectacles/tracking.py | 50 ---------------- tests/unit/test_cli.py | 55 +---------------- tests/unit/test_tracking.py | 116 ------------------------------------ 4 files changed, 1 insertion(+), 242 deletions(-) delete mode 100644 spectacles/tracking.py delete mode 100644 tests/unit/test_tracking.py diff --git a/spectacles/cli.py b/spectacles/cli.py index 97388064..313f9848 100644 --- a/spectacles/cli.py +++ b/spectacles/cli.py @@ -16,7 +16,6 @@ from yaml.parser import ParserError import spectacles.printer as printer -import spectacles.tracking as tracking from spectacles.client import ( DEFAULT_API_VERSION, LOOKML_VALIDATION_TIMEOUT, @@ -292,13 +291,6 @@ def main() -> None: set_file_handler(args.log_dir) - if not args.do_not_track: - invocation_id = tracking.track_invocation_start( - args.base_url, - args.command, - project=args.project if args.command != "connect" else None, - ) - if args.command == "connect": asyncio.run( run_connect( @@ -389,14 +381,6 @@ def main() -> None: ) ) - if not args.do_not_track: - tracking.track_invocation_end( - args.base_url, - args.command, - invocation_id, # pyright: ignore[reportUnboundVariable, reportPossiblyUnboundVariable] - args.project if args.command != "connect" else None, - ) - def create_parser() -> ArgumentParser: """Creates the top-level argument parser. @@ -486,12 +470,6 @@ def _build_base_subparser() -> ArgumentParser: default="logs", help="The directory that Spectacles will write logs to.", ) - base_subparser.add_argument( - "--do-not-track", - action=EnvVarStoreTrueAction, - env_var="SPECTACLES_DO_NOT_TRACK", - help="Disables anonymised event tracking.", - ) return base_subparser diff --git a/spectacles/tracking.py b/spectacles/tracking.py deleted file mode 100644 index 30cdac0f..00000000 --- a/spectacles/tracking.py +++ /dev/null @@ -1,50 +0,0 @@ -import hashlib -import uuid -from typing import Optional - -import segment.analytics as analytics - -analytics.write_key = "QnqzXWlqkmgDSm7X2qFDrxx3LGCW7Rba" - - -def anonymise(trait: str) -> str: - trait = hashlib.md5(trait.encode()).hexdigest() # nosec - return trait - - -def track_invocation_start( - base_url: str, - command: str, - invocation_id: str = str(uuid.uuid4()), - project: Optional[str] = None, -) -> str: - url_hash = anonymise(base_url.rstrip("/")) - project_hash = anonymise(project) if project else None - analytics.track( - user_id=url_hash, - event="invocation", - properties={ - "label": "start", - "command": command, - "project": project_hash, - "invocation_id": invocation_id, - }, - ) - return invocation_id - - -def track_invocation_end( - base_url: str, command: str, invocation_id: str, project: Optional[str] = None -) -> None: - url_hash = anonymise(base_url.rstrip("/")) - project_hash = anonymise(project) if project else None - analytics.track( - user_id=url_hash, - event="invocation", - properties={ - "label": "end", - "command": command, - "project": project_hash, - "invocation_id": invocation_id, - }, - ) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index d1d6ae9b..f79fc0d8 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -343,9 +343,7 @@ def test_parse_args_with_mutually_exclusive_args_commit_ref( @patch("sys.argv", new=["spectacles", "sql"]) @patch("spectacles.cli.Runner") @patch("spectacles.cli.LookerClient", autospec=True) -@patch("spectacles.cli.tracking") def test_main_with_sql_validator( - mock_tracking: MagicMock, mock_client: MagicMock, mock_runner: MagicMock, env: None, @@ -355,11 +353,6 @@ def test_main_with_sql_validator( mock_runner.return_value.validate_sql = AsyncMock(return_value=validation) with pytest.raises(SystemExit): main() - mock_tracking.track_invocation_start.assert_called_once_with( - "BASE_URL_ENV_VAR", "sql", project="PROJECT_ENV_VAR" - ) - # TODO: Uncomment the below assertion once #262 is fixed - # mock_tracking.track_invocation_end.assert_called_once() mock_runner.assert_called_once() assert "ecommerce.orders passed" in caplog.text assert "ecommerce.sessions passed" in caplog.text @@ -369,9 +362,7 @@ def test_main_with_sql_validator( @patch("sys.argv", new=["spectacles", "content"]) @patch("spectacles.cli.Runner") @patch("spectacles.cli.LookerClient", autospec=True) -@patch("spectacles.cli.tracking") def test_main_with_content_validator( - mock_tracking: MagicMock, mock_client: MagicMock, mock_runner: MagicMock, env: None, @@ -381,11 +372,6 @@ def test_main_with_content_validator( mock_runner.return_value.validate_content = AsyncMock(return_value=validation) with pytest.raises(SystemExit): main() - mock_tracking.track_invocation_start.assert_called_once_with( - "BASE_URL_ENV_VAR", "content", project="PROJECT_ENV_VAR" - ) - # TODO: Uncomment the below assertion once #262 is fixed - # mock_tracking.track_invocation_end.assert_called_once() mock_runner.assert_called_once() assert "ecommerce.orders passed" in caplog.text assert "ecommerce.sessions passed" in caplog.text @@ -395,9 +381,7 @@ def test_main_with_content_validator( @patch("sys.argv", new=["spectacles", "assert"]) @patch("spectacles.cli.Runner", autospec=True) @patch("spectacles.cli.LookerClient", autospec=True) -@patch("spectacles.cli.tracking") def test_main_with_assert_validator( - mock_tracking: MagicMock, mock_client: MagicMock, mock_runner: MagicMock, env: None, @@ -407,11 +391,6 @@ def test_main_with_assert_validator( mock_runner.return_value.validate_data_tests = AsyncMock(return_value=validation) with pytest.raises(SystemExit): main() - mock_tracking.track_invocation_start.assert_called_once_with( - "BASE_URL_ENV_VAR", "assert", project="PROJECT_ENV_VAR" - ) - # TODO: Uncomment the below assertion once #262 is fixed - # mock_tracking.track_invocation_end.assert_called_once() mock_runner.assert_called_once() assert "ecommerce.orders passed" in caplog.text assert "ecommerce.sessions passed" in caplog.text @@ -421,9 +400,7 @@ def test_main_with_assert_validator( @patch("sys.argv", new=["spectacles", "lookml"]) @patch("spectacles.cli.Runner", autospec=True) @patch("spectacles.cli.LookerClient", autospec=True) -@patch("spectacles.cli.tracking") def test_main_with_lookml_validator( - mock_tracking: MagicMock, mock_client: MagicMock, mock_runner: MagicMock, env: None, @@ -433,11 +410,6 @@ def test_main_with_lookml_validator( mock_runner.return_value.validate_lookml = AsyncMock(return_value=validation) with pytest.raises(SystemExit): main() - mock_tracking.track_invocation_start.assert_called_once_with( - "BASE_URL_ENV_VAR", "lookml", project="PROJECT_ENV_VAR" - ) - # TODO: Uncomment the below assertion once #262 is fixed - # mock_tracking.track_invocation_end.assert_called_once() mock_runner.assert_called_once() assert "eye_exam/eye_exam.model.lkml" in caplog.text assert "Could not find a field named 'users__fail.first_name'" in caplog.text @@ -445,33 +417,8 @@ def test_main_with_lookml_validator( @patch("sys.argv", new=["spectacles", "connect"]) @patch("spectacles.cli.run_connect") -@patch("spectacles.cli.tracking") -def test_main_with_connect( - mock_tracking: MagicMock, mock_run_connect: AsyncMock, env: None -) -> None: - main() - mock_tracking.track_invocation_start.assert_called_once_with( - "BASE_URL_ENV_VAR", "connect", project=None - ) - mock_tracking.track_invocation_end.assert_called_once() - mock_run_connect.assert_called_once_with( - base_url="BASE_URL_ENV_VAR", - client_id="CLIENT_ID_ENV_VAR", - client_secret="CLIENT_SECRET_ENV_VAR", - port=8080, - api_version=4.0, - ) - - -@patch("sys.argv", new=["spectacles", "connect", "--do-not-track"]) -@patch("spectacles.cli.run_connect") -@patch("spectacles.cli.tracking") -def test_main_with_do_not_track( - mock_tracking: MagicMock, mock_run_connect: AsyncMock, env: None -) -> None: +def test_main_with_connect(mock_run_connect: AsyncMock, env: None) -> None: main() - mock_tracking.track_invocation_start.assert_not_called() - mock_tracking.track_invocation_end.assert_not_called() mock_run_connect.assert_called_once_with( base_url="BASE_URL_ENV_VAR", client_id="CLIENT_ID_ENV_VAR", diff --git a/tests/unit/test_tracking.py b/tests/unit/test_tracking.py deleted file mode 100644 index 2285f6ee..00000000 --- a/tests/unit/test_tracking.py +++ /dev/null @@ -1,116 +0,0 @@ -from unittest.mock import MagicMock, patch - -from spectacles import tracking - - -def test_anonymise_url() -> None: - hashed_url = tracking.anonymise("https://organisation.looker.com") - assert hashed_url == "67d1b18410d23b5765caa3320b703154" - - -def test_anonymise_project() -> None: - hashed_url = tracking.anonymise("test_project") - assert hashed_url == "6e72a69d5c5cca8f0400338441c022e4" - - -@patch("segment.analytics.track") -def test_track_invocation_start_sql(mock_track: MagicMock) -> None: - invocation_id = tracking.track_invocation_start( - "https://organisation.looker.com", "sql", "123456", "test_project" - ) - mock_track.assert_called_once_with( - user_id="67d1b18410d23b5765caa3320b703154", - event="invocation", - properties={ - "label": "start", - "command": "sql", - "project": "6e72a69d5c5cca8f0400338441c022e4", - "invocation_id": "123456", - }, - ) - assert invocation_id == "123456" - - -@patch("segment.analytics.track") -def test_track_invocation_start_assert(mock_track: MagicMock) -> None: - invocation_id = tracking.track_invocation_start( - "https://organisation.looker.com", "assert", "123456", "test_project" - ) - mock_track.assert_called_once_with( - user_id="67d1b18410d23b5765caa3320b703154", - event="invocation", - properties={ - "label": "start", - "command": "assert", - "project": "6e72a69d5c5cca8f0400338441c022e4", - "invocation_id": "123456", - }, - ) - assert invocation_id == "123456" - - -@patch("segment.analytics.track") -def test_track_invocation_start_connect(mock_track: MagicMock) -> None: - invocation_id = tracking.track_invocation_start( - "https://organisation.looker.com", "assert", "123456" - ) - mock_track.assert_called_once_with( - user_id="67d1b18410d23b5765caa3320b703154", - event="invocation", - properties={ - "label": "start", - "command": "assert", - "project": None, - "invocation_id": "123456", - }, - ) - assert invocation_id == "123456" - - -@patch("segment.analytics.track") -def test_track_invocation_end_sql(mock_track: MagicMock) -> None: - tracking.track_invocation_end( - "https://organisation.looker.com", "sql", "123456", "test_project" - ) - mock_track.assert_called_once_with( - user_id="67d1b18410d23b5765caa3320b703154", - event="invocation", - properties={ - "label": "end", - "command": "sql", - "project": "6e72a69d5c5cca8f0400338441c022e4", - "invocation_id": "123456", - }, - ) - - -@patch("segment.analytics.track") -def test_track_invocation_end_assert(mock_track: MagicMock) -> None: - tracking.track_invocation_end( - "https://organisation.looker.com", "assert", "123456", "test_project" - ) - mock_track.assert_called_once_with( - user_id="67d1b18410d23b5765caa3320b703154", - event="invocation", - properties={ - "label": "end", - "command": "assert", - "project": "6e72a69d5c5cca8f0400338441c022e4", - "invocation_id": "123456", - }, - ) - - -@patch("segment.analytics.track") -def test_track_invocation_end_connect(mock_track: MagicMock) -> None: - tracking.track_invocation_end("https://organisation.looker.com", "assert", "123456") - mock_track.assert_called_once_with( - user_id="67d1b18410d23b5765caa3320b703154", - event="invocation", - properties={ - "label": "end", - "command": "assert", - "project": None, - "invocation_id": "123456", - }, - ) From bd7be358f5ad62c03a4109374e9d1ddf2955a00c Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Mon, 2 Dec 2024 15:38:41 +0000 Subject: [PATCH 2/4] bump Spectacles version 2.4.10 -> 2.4.11 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8c6f4863..c6f7ba2e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "spectacles" -version = "2.4.10" +version = "3.0.0" description = "A command-line, continuous integration tool for Looker and LookML." authors = ["Spectacles "] license = "MIT" From 847151da671390079f91b76772b585df51b07af9 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Mon, 2 Dec 2024 15:38:46 +0000 Subject: [PATCH 3/4] bump Spectacles version 2.4.10 -> 2.4.11 --- pyproject.toml | 2 +- spectacles/cli.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c6f7ba2e..5bea2f8c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "spectacles" -version = "3.0.0" +version = "2.4.11" description = "A command-line, continuous integration tool for Looker and LookML." authors = ["Spectacles "] license = "MIT" diff --git a/spectacles/cli.py b/spectacles/cli.py index 313f9848..09cb17e6 100644 --- a/spectacles/cli.py +++ b/spectacles/cli.py @@ -470,6 +470,12 @@ def _build_base_subparser() -> ArgumentParser: default="logs", help="The directory that Spectacles will write logs to.", ) + base_subparser.add_argument( + "--do-not-track", + action=EnvVarStoreTrueAction, + env_var="SPECTACLES_DO_NOT_TRACK", + help="[LEGACY] This argument is no longer used.", + ) return base_subparser From 969a7bb5c08c1bf09c43b5723a746cfd3e66871e Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Mon, 2 Dec 2024 15:47:56 +0000 Subject: [PATCH 4/4] Update spectacles/cli.py Co-authored-by: Josh Temple <8672171+joshtemple@users.noreply.github.com> --- spectacles/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/cli.py b/spectacles/cli.py index 09cb17e6..a0343202 100644 --- a/spectacles/cli.py +++ b/spectacles/cli.py @@ -474,7 +474,7 @@ def _build_base_subparser() -> ArgumentParser: "--do-not-track", action=EnvVarStoreTrueAction, env_var="SPECTACLES_DO_NOT_TRACK", - help="[LEGACY] This argument is no longer used.", + help="[DEPRECATED] Tracking is disabled by default, this argument is unused.", ) return base_subparser