-
Notifications
You must be signed in to change notification settings - Fork 0
feat(python-sdk): add logger, datastore queue parity, and docs refresh #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,16 @@ | ||
| # Convert Python SDK | ||
|
|
||
| This package contains the first parity-focused slice of the Python Convert SDK: | ||
| This package is the Python port of the Convert Fullstack SDK. | ||
|
|
||
| Current functionality includes: | ||
|
|
||
| - deterministic MurmurHash3-based bucketing | ||
| - rule comparisons | ||
| - rule traversal | ||
| - JS-equivalent rule comparisons and traversal | ||
| - static-config and `sdkKey` initialization | ||
| - experience and feature evaluation through visitor contexts | ||
| - conversion tracking and API queue release | ||
| - in-memory and datastore-backed visitor persistence | ||
| - helper `DataStore` and `FileLogger` utilities | ||
| - configurable logger clients and log levels | ||
|
|
||
| Networking, tracking, config fetching, and the public SDK surface are intentionally left for follow-up PRs. | ||
| The SDK entrypoint is `ConvertSDK`, which can create visitor contexts and evaluate the same CDN config shape used by the JavaScript SDK. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,27 +1,122 @@ | ||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import threading | ||||||||||||||||||||||||||||||||||
| from collections.abc import Mapping | ||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from convertcom_sdk.enums import SystemEvents | ||||||||||||||||||||||||||||||||||
| from convertcom_sdk.utils.object_utils import object_deep_merge | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| DEFAULT_BATCH_SIZE = 1 | ||||||||||||||||||||||||||||||||||
| DEFAULT_RELEASE_INTERVAL = 5000 | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class DataStoreManager: | ||||||||||||||||||||||||||||||||||
| def __init__( | ||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||
| config: Mapping[str, Any] | None = None, | ||||||||||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||||||||||
| data_store: Any = None, | ||||||||||||||||||||||||||||||||||
| event_manager: Any | None = None, | ||||||||||||||||||||||||||||||||||
| logger_manager: Any | None = None, | ||||||||||||||||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||||||||||||||||
| del config | ||||||||||||||||||||||||||||||||||
| config = dict(config or {}) | ||||||||||||||||||||||||||||||||||
| events = config.get("events") or {} | ||||||||||||||||||||||||||||||||||
| self._lock = threading.RLock() | ||||||||||||||||||||||||||||||||||
| self._event_manager = event_manager | ||||||||||||||||||||||||||||||||||
| self._logger_manager = logger_manager | ||||||||||||||||||||||||||||||||||
| self._mapper = config.get("mapper") or (lambda value: value) | ||||||||||||||||||||||||||||||||||
| self.batch_size = int(events.get("batch_size") or DEFAULT_BATCH_SIZE) | ||||||||||||||||||||||||||||||||||
| self.release_interval = int( | ||||||||||||||||||||||||||||||||||
| events.get("release_interval") or DEFAULT_RELEASE_INTERVAL | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| self._requests_queue: dict[str, Any] = {} | ||||||||||||||||||||||||||||||||||
| self._requests_queue_timer: threading.Timer | None = None | ||||||||||||||||||||||||||||||||||
| self.data_store = data_store if self.is_valid_data_store(data_store) else None | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def set(self, key: str, data: Any) -> None: | ||||||||||||||||||||||||||||||||||
| if self.data_store is not None: | ||||||||||||||||||||||||||||||||||
| if self.data_store is None: | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||
| self.data_store.set(key, data) | ||||||||||||||||||||||||||||||||||
| except Exception as error: | ||||||||||||||||||||||||||||||||||
| if self._logger_manager: | ||||||||||||||||||||||||||||||||||
| self._logger_manager.error( | ||||||||||||||||||||||||||||||||||
| "DataStoreManager.set()", | ||||||||||||||||||||||||||||||||||
| {"error": str(error)}, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def get(self, key: str) -> Any: | ||||||||||||||||||||||||||||||||||
| if self.data_store is None: | ||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||
| return self.data_store.get(key) | ||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||
| return self.data_store.get(key) | ||||||||||||||||||||||||||||||||||
| except Exception as error: | ||||||||||||||||||||||||||||||||||
| if self._logger_manager: | ||||||||||||||||||||||||||||||||||
| self._logger_manager.error( | ||||||||||||||||||||||||||||||||||
| "DataStoreManager.get()", | ||||||||||||||||||||||||||||||||||
| {"error": str(error)}, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def enqueue(self, key: str, data: Any) -> None: | ||||||||||||||||||||||||||||||||||
| if self._logger_manager: | ||||||||||||||||||||||||||||||||||
| self._logger_manager.trace( | ||||||||||||||||||||||||||||||||||
| "DataStoreManager.enqueue()", | ||||||||||||||||||||||||||||||||||
| self._mapper({"key": key, "data": data}), | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| with self._lock: | ||||||||||||||||||||||||||||||||||
| self._requests_queue = object_deep_merge( | ||||||||||||||||||||||||||||||||||
| self._requests_queue, | ||||||||||||||||||||||||||||||||||
| {key: data}, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| queue_length = len(self._requests_queue) | ||||||||||||||||||||||||||||||||||
| if queue_length >= self.batch_size: | ||||||||||||||||||||||||||||||||||
| self.release_queue("size") | ||||||||||||||||||||||||||||||||||
| elif queue_length == 1: | ||||||||||||||||||||||||||||||||||
| self.start_queue() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def release_queue(self, reason: str | None = None) -> None: | ||||||||||||||||||||||||||||||||||
| if self._logger_manager: | ||||||||||||||||||||||||||||||||||
| self._logger_manager.info( | ||||||||||||||||||||||||||||||||||
| "DataStoreManager.release_queue()", | ||||||||||||||||||||||||||||||||||
| {"reason": reason or ""}, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| with self._lock: | ||||||||||||||||||||||||||||||||||
| queued_items = dict(self._requests_queue) | ||||||||||||||||||||||||||||||||||
| self._requests_queue = {} | ||||||||||||||||||||||||||||||||||
| self.stop_queue() | ||||||||||||||||||||||||||||||||||
| for key, value in queued_items.items(): | ||||||||||||||||||||||||||||||||||
| self.set(key, value) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+89
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling
Suggested change
|
||||||||||||||||||||||||||||||||||
| if self._event_manager: | ||||||||||||||||||||||||||||||||||
| self._event_manager.fire( | ||||||||||||||||||||||||||||||||||
| SystemEvents.DATA_STORE_QUEUE_RELEASED, | ||||||||||||||||||||||||||||||||||
| {"reason": reason or ""}, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def releaseQueue(self, reason: str | None = None) -> None: | ||||||||||||||||||||||||||||||||||
| self.release_queue(reason) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+97
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I understand the goal of achieving parity with the JS SDK, adding camelCase aliases for Python methods goes against PEP 8, which recommends snake_case for functions and methods. This can be confusing for Python developers and harm maintainability. It's best to stick to idiomatic Python and remove these aliases. |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def stop_queue(self) -> None: | ||||||||||||||||||||||||||||||||||
| with self._lock: | ||||||||||||||||||||||||||||||||||
| timer = self._requests_queue_timer | ||||||||||||||||||||||||||||||||||
| self._requests_queue_timer = None | ||||||||||||||||||||||||||||||||||
| if timer: | ||||||||||||||||||||||||||||||||||
| timer.cancel() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def start_queue(self) -> None: | ||||||||||||||||||||||||||||||||||
| self.stop_queue() | ||||||||||||||||||||||||||||||||||
| with self._lock: | ||||||||||||||||||||||||||||||||||
| timer = threading.Timer( | ||||||||||||||||||||||||||||||||||
| self.release_interval / 1000.0, | ||||||||||||||||||||||||||||||||||
| lambda: self.release_queue("timeout"), | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| timer.daemon = True | ||||||||||||||||||||||||||||||||||
| self._requests_queue_timer = timer | ||||||||||||||||||||||||||||||||||
| timer.start() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def close(self) -> None: | ||||||||||||||||||||||||||||||||||
| self.stop_queue() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def is_valid_data_store(self, data_store: Any) -> bool: | ||||||||||||||||||||||||||||||||||
| return bool( | ||||||||||||||||||||||||||||||||||
|
|
@@ -31,12 +126,3 @@ def is_valid_data_store(self, data_store: Any) -> bool: | |||||||||||||||||||||||||||||||||
| and hasattr(data_store, "set") | ||||||||||||||||||||||||||||||||||
| and callable(data_store.set) | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def release_queue(self, reason: str | None = None) -> Any: | ||||||||||||||||||||||||||||||||||
| if self.data_store is None: | ||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||
| if hasattr(self.data_store, "release_queue") and callable(self.data_store.release_queue): | ||||||||||||||||||||||||||||||||||
| return self.data_store.release_queue(reason) | ||||||||||||||||||||||||||||||||||
| if hasattr(self.data_store, "releaseQueue") and callable(self.data_store.releaseQueue): | ||||||||||||||||||||||||||||||||||
| return self.data_store.releaseQueue(reason) | ||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| from .log_manager import LogManager | ||
|
|
||
| __all__ = ["LogManager"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential bug here. If
new_datacontains only non-reportable segments,new_segmentswill be empty, and theelsebranch will be taken. This setsstored_valuetoupdated, which includes the non-reportable segments. This seems to contradict the goal of persisting only reportable segments. Thestored_valueshould always be constructed using only filtered, reportable segments.