From 7728f6ba933a694617a895b123f1e2cda2e5a19f Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Thu, 17 Jul 2025 14:55:31 -0700 Subject: [PATCH 1/7] fix ph initialization --- src/ploomber_core/telemetry/telemetry.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/ploomber_core/telemetry/telemetry.py b/src/ploomber_core/telemetry/telemetry.py index 72d3f95..3dd298a 100644 --- a/src/ploomber_core/telemetry/telemetry.py +++ b/src/ploomber_core/telemetry/telemetry.py @@ -418,6 +418,14 @@ def __init__(self, api_key, package_name, version, *, print_cloud_message=True): self.package_name = package_name self.version = version self.print_cloud_message = print_cloud_message + + # Initialize PostHog client properly + try: + self._posthog_client = posthog.Posthog(api_key, host='https://us.i.posthog.com') + except Exception as e: + # If PostHog initialization fails, create a dummy client + warnings.warn(f"Failed to initialize PostHog client: {e}") + self._posthog_client = None @classmethod def from_package(cls, package_name, *, print_cloud_message=True, api_key=None): @@ -440,7 +448,6 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): if missing like timestamp, event id and stats information. """ - posthog.project_api_key = self.api_key metadata = metadata or {} event_id = uuid4() @@ -516,12 +523,15 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): "metadata": metadata, } - if is_install: - posthog.capture( - distinct_id=uid, event="install_success_indirect", properties=props - ) + if self._posthog_client is not None: + if is_install: + self._posthog_client.capture( + distinct_id=uid, event="install_success_indirect", properties=props + ) - posthog.capture(distinct_id=uid, event=action, properties=props) + self._posthog_client.capture(distinct_id=uid, event=action, properties=props) + else: + raise RuntimeError("Log call failed: PostHog client not initialized.") # NOTE: should we log differently depending on the error type? # NOTE: how should we handle chained exceptions? From 2f98c79dca0196922e40591a922763ce039b37bd Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Thu, 17 Jul 2025 15:00:28 -0700 Subject: [PATCH 2/7] lint --- src/ploomber_core/telemetry/telemetry.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ploomber_core/telemetry/telemetry.py b/src/ploomber_core/telemetry/telemetry.py index 3dd298a..918d30d 100644 --- a/src/ploomber_core/telemetry/telemetry.py +++ b/src/ploomber_core/telemetry/telemetry.py @@ -418,10 +418,12 @@ def __init__(self, api_key, package_name, version, *, print_cloud_message=True): self.package_name = package_name self.version = version self.print_cloud_message = print_cloud_message - - # Initialize PostHog client properly + + # Initialize Posthog client try: - self._posthog_client = posthog.Posthog(api_key, host='https://us.i.posthog.com') + self._posthog_client = posthog.Posthog( + api_key, host="https://us.i.posthog.com" + ) except Exception as e: # If PostHog initialization fails, create a dummy client warnings.warn(f"Failed to initialize PostHog client: {e}") @@ -526,10 +528,14 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): if self._posthog_client is not None: if is_install: self._posthog_client.capture( - distinct_id=uid, event="install_success_indirect", properties=props + distinct_id=uid, + event="install_success_indirect", + properties=props, ) - self._posthog_client.capture(distinct_id=uid, event=action, properties=props) + self._posthog_client.capture( + distinct_id=uid, event=action, properties=props + ) else: raise RuntimeError("Log call failed: PostHog client not initialized.") From 5bfa06d6406ede1833293642b4eec0f6137e6798 Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Thu, 17 Jul 2025 15:10:11 -0700 Subject: [PATCH 3/7] pin posthog instead of breaking changes --- setup.py | 2 +- src/ploomber_core/telemetry/telemetry.py | 30 +++++++----------------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/setup.py b/setup.py index e5c2a7d..d11fbb8 100644 --- a/setup.py +++ b/setup.py @@ -15,7 +15,7 @@ REQUIRES = [ "pyyaml", - "posthog", + "posthog<3.0", 'importlib-metadata;python_version<"3.8"', ] diff --git a/src/ploomber_core/telemetry/telemetry.py b/src/ploomber_core/telemetry/telemetry.py index 918d30d..4312170 100644 --- a/src/ploomber_core/telemetry/telemetry.py +++ b/src/ploomber_core/telemetry/telemetry.py @@ -419,16 +419,6 @@ def __init__(self, api_key, package_name, version, *, print_cloud_message=True): self.version = version self.print_cloud_message = print_cloud_message - # Initialize Posthog client - try: - self._posthog_client = posthog.Posthog( - api_key, host="https://us.i.posthog.com" - ) - except Exception as e: - # If PostHog initialization fails, create a dummy client - warnings.warn(f"Failed to initialize PostHog client: {e}") - self._posthog_client = None - @classmethod def from_package(cls, package_name, *, print_cloud_message=True, api_key=None): """ @@ -450,6 +440,9 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): if missing like timestamp, event id and stats information. """ + # This method of setting the API Key is deprecated in PostHog 3.x + # PostHog has been pinned to 2.x to avoid breaking changes + posthog.project_api_key = self.api_key metadata = metadata or {} event_id = uuid4() @@ -525,19 +518,12 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): "metadata": metadata, } - if self._posthog_client is not None: - if is_install: - self._posthog_client.capture( - distinct_id=uid, - event="install_success_indirect", - properties=props, - ) - - self._posthog_client.capture( - distinct_id=uid, event=action, properties=props + if is_install: + posthog.capture( + distinct_id=uid, event="install_success_indirect", properties=props ) - else: - raise RuntimeError("Log call failed: PostHog client not initialized.") + + posthog.capture(distinct_id=uid, event=action, properties=props) # NOTE: should we log differently depending on the error type? # NOTE: how should we handle chained exceptions? From db7e73cfc18c906c49530e717c4110f8ad110937 Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Thu, 17 Jul 2025 15:22:13 -0700 Subject: [PATCH 4/7] update rtd, pin ph, bring back changes --- .github/workflows/ci.yaml | 2 +- .readthedocs.yml | 3 ++- setup.py | 2 +- src/ploomber_core/telemetry/telemetry.py | 24 ++++++++++++++++-------- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index de76528..b599e86 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -6,7 +6,7 @@ jobs: test: strategy: matrix: - python-version: [3.8, 3.9, '3.10', '3.11'] + python-version: ['3.10', '3.11', '3.12'] os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} diff --git a/.readthedocs.yml b/.readthedocs.yml index 99333c8..c45b822 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -14,4 +14,5 @@ conda: sphinx: builder: html - fail_on_warning: true \ No newline at end of file + fail_on_warning: true + configuration: doc/_config.yml \ No newline at end of file diff --git a/setup.py b/setup.py index d11fbb8..d514fe8 100644 --- a/setup.py +++ b/setup.py @@ -15,7 +15,7 @@ REQUIRES = [ "pyyaml", - "posthog<3.0", + "posthog>=3.0", 'importlib-metadata;python_version<"3.8"', ] diff --git a/src/ploomber_core/telemetry/telemetry.py b/src/ploomber_core/telemetry/telemetry.py index 4312170..f2d5014 100644 --- a/src/ploomber_core/telemetry/telemetry.py +++ b/src/ploomber_core/telemetry/telemetry.py @@ -419,6 +419,14 @@ def __init__(self, api_key, package_name, version, *, print_cloud_message=True): self.version = version self.print_cloud_message = print_cloud_message + # Initialize PostHog client + try: + self._posthog_client = posthog.Posthog(api_key, host='https://us.i.posthog.com') + except Exception as e: + # If PostHog initialization fails, create a dummy client + warnings.warn(f"Failed to initialize PostHog client: {e}") + self._posthog_client = None + @classmethod def from_package(cls, package_name, *, print_cloud_message=True, api_key=None): """ @@ -440,9 +448,6 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): if missing like timestamp, event id and stats information. """ - # This method of setting the API Key is deprecated in PostHog 3.x - # PostHog has been pinned to 2.x to avoid breaking changes - posthog.project_api_key = self.api_key metadata = metadata or {} event_id = uuid4() @@ -518,12 +523,15 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): "metadata": metadata, } - if is_install: - posthog.capture( - distinct_id=uid, event="install_success_indirect", properties=props - ) + if self._posthog_client is not None: + if is_install: + self._posthog_client.capture( + distinct_id=uid, event="install_success_indirect", properties=props + ) - posthog.capture(distinct_id=uid, event=action, properties=props) + self._posthog_client.capture(distinct_id=uid, event=action, properties=props) + else: + raise RuntimeError("Log call failed: PostHog client not initialized") # NOTE: should we log differently depending on the error type? # NOTE: how should we handle chained exceptions? From 7654dc6ba4d8cabb2e1d3040ee7ed8e3c75639c7 Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Thu, 17 Jul 2025 15:23:12 -0700 Subject: [PATCH 5/7] lint --- src/ploomber_core/telemetry/telemetry.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/ploomber_core/telemetry/telemetry.py b/src/ploomber_core/telemetry/telemetry.py index f2d5014..826d3ef 100644 --- a/src/ploomber_core/telemetry/telemetry.py +++ b/src/ploomber_core/telemetry/telemetry.py @@ -421,7 +421,9 @@ def __init__(self, api_key, package_name, version, *, print_cloud_message=True): # Initialize PostHog client try: - self._posthog_client = posthog.Posthog(api_key, host='https://us.i.posthog.com') + self._posthog_client = posthog.Posthog( + api_key, host="https://us.i.posthog.com" + ) except Exception as e: # If PostHog initialization fails, create a dummy client warnings.warn(f"Failed to initialize PostHog client: {e}") @@ -526,10 +528,14 @@ def log_api(self, action, client_time=None, total_runtime=None, metadata=None): if self._posthog_client is not None: if is_install: self._posthog_client.capture( - distinct_id=uid, event="install_success_indirect", properties=props + distinct_id=uid, + event="install_success_indirect", + properties=props, ) - self._posthog_client.capture(distinct_id=uid, event=action, properties=props) + self._posthog_client.capture( + distinct_id=uid, event=action, properties=props + ) else: raise RuntimeError("Log call failed: PostHog client not initialized") From 6c25ef41442b4bb1f595ec9099d29cb80a1ad57e Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Thu, 17 Jul 2025 15:35:10 -0700 Subject: [PATCH 6/7] fix mocks --- tests/conftest.py | 7 +++++- tests/telemetry/test_telemetry.py | 26 ++++++++++++++++------ tests/telemetry/test_telemetry_log_args.py | 10 ++++++--- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index f09290d..9030f5f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -83,5 +83,10 @@ def external_access(request, monkeypatch_session): # https://github.com/pytest-dev/pytest/issues/7061#issuecomment-611892868 external_access = MagicMock() external_access.get_something = MagicMock(return_value="Mock was used.") - monkeypatch_session.setattr(posthog, "capture", external_access.get_something) + + mock_posthog_instance = MagicMock() + mock_posthog_instance.capture = external_access.get_something + mock_posthog_class = MagicMock(return_value=mock_posthog_instance) + + monkeypatch_session.setattr(posthog, "Posthog", mock_posthog_class) yield diff --git a/tests/telemetry/test_telemetry.py b/tests/telemetry/test_telemetry.py index f063556..2ddf791 100644 --- a/tests/telemetry/test_telemetry.py +++ b/tests/telemetry/test_telemetry.py @@ -826,7 +826,11 @@ def fake_capture(*args, **kwargs): log = logging.getLogger("posthog") log.error("some error happened") - monkeypatch.setattr(posthog, "capture", fake_capture) + mock_posthog_instance = Mock() + mock_posthog_instance.capture = fake_capture + mock_posthog_class = Mock(return_value=mock_posthog_instance) + + monkeypatch.setattr(posthog, "Posthog", mock_posthog_class) _telemetry = telemetry.Telemetry(MOCK_API_KEY, "ploomber", "0.14.0") with caplog.at_level(logging.ERROR, logger="posthog"): @@ -838,8 +842,12 @@ def fake_capture(*args, **kwargs): # TODO: test more of the values (I'm adding ANY to many of them) def test_log_api_stored_values(monkeypatch): mock_info = Mock(return_value=(True, "fake-uuid", False)) - mock = Mock() - monkeypatch.setattr(telemetry.posthog, "capture", mock) + mock_capture = Mock() + mock_posthog_instance = Mock() + mock_posthog_instance.capture = mock_capture + mock_posthog_class = Mock(return_value=mock_posthog_instance) + + monkeypatch.setattr(telemetry.posthog, "Posthog", mock_posthog_class) monkeypatch.setattr(telemetry, "_get_telemetry_info", mock_info) _telemetry = telemetry.Telemetry(MOCK_API_KEY, "some-package", "1.2.2") @@ -851,7 +859,7 @@ def test_log_api_stored_values(monkeypatch): f"{sys.version_info.micro}" ) - mock.assert_called_once_with( + mock_capture.assert_called_once_with( distinct_id="fake-uuid", event="some-action", properties={ @@ -876,8 +884,12 @@ def test_log_api_stored_values(monkeypatch): def test_log_call_stored_values(monkeypatch): mock_info = Mock(return_value=(True, "fake-uuid", False)) - mock = Mock() - monkeypatch.setattr(telemetry.posthog, "capture", mock) + mock_capture = Mock() + mock_posthog_instance = Mock() + mock_posthog_instance.capture = mock_capture + mock_posthog_class = Mock(return_value=mock_posthog_instance) + + monkeypatch.setattr(telemetry.posthog, "Posthog", mock_posthog_class) monkeypatch.setattr(telemetry, "_get_telemetry_info", mock_info) monkeypatch.setattr(telemetry.sys, "argv", ["/path/to/bin", "arg2", "arg2"]) @@ -894,7 +906,7 @@ def my_function(): f"{sys.version_info.micro}" ) - assert mock.call_args_list == [ + assert mock_capture.call_args_list == [ call( distinct_id="fake-uuid", event="some-package-some-action-success", diff --git a/tests/telemetry/test_telemetry_log_args.py b/tests/telemetry/test_telemetry_log_args.py index 8b339ee..af7a884 100644 --- a/tests/telemetry/test_telemetry_log_args.py +++ b/tests/telemetry/test_telemetry_log_args.py @@ -7,10 +7,14 @@ @pytest.fixture def mock_posthog(monkeypatch): mock_info = Mock(return_value=(True, "UUID", False)) - mock = Mock() - monkeypatch.setattr(telemetry_module.posthog, "capture", mock) + mock_capture = Mock() + mock_posthog_instance = Mock() + mock_posthog_instance.capture = mock_capture + mock_posthog_class = Mock(return_value=mock_posthog_instance) + + monkeypatch.setattr(telemetry_module.posthog, "Posthog", mock_posthog_class) monkeypatch.setattr(telemetry_module, "_get_telemetry_info", mock_info) - yield mock + yield mock_capture @pytest.mark.parametrize( From ca6301a156641fd56a107e76a86e8f3f3b6999a9 Mon Sep 17 00:00:00 2001 From: Bryan Ho Date: Mon, 21 Jul 2025 09:13:31 -0700 Subject: [PATCH 7/7] fix incompatibility message --- src/ploomber_core/telemetry/telemetry.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ploomber_core/telemetry/telemetry.py b/src/ploomber_core/telemetry/telemetry.py index 826d3ef..ef62ba7 100644 --- a/src/ploomber_core/telemetry/telemetry.py +++ b/src/ploomber_core/telemetry/telemetry.py @@ -425,9 +425,11 @@ def __init__(self, api_key, package_name, version, *, print_cloud_message=True): api_key, host="https://us.i.posthog.com" ) except Exception as e: - # If PostHog initialization fails, create a dummy client - warnings.warn(f"Failed to initialize PostHog client: {e}") - self._posthog_client = None + raise ImportError( + "Failed to initialize posthog client. This likely means your posthog " + "version is incompatible. To fix this, either upgrade to posthog>=3.0 " + "or downgrade to ploomber-core<=0.2.26" + ) from e @classmethod def from_package(cls, package_name, *, print_cloud_message=True, api_key=None):