Skip to content

feat: add odp manager #405

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
Sep 21, 2022
Merged

feat: add odp manager #405

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f97f940
bump up py version in gitactions to 3.10
Mat001 Aug 26, 2022
a26e459
merge conflict resolve
Mat001 Aug 29, 2022
d2ed75e
Merge branch 'master' of github.com:optimizely/python-sdk
Mat001 Sep 2, 2022
77dfec4
feat: add first draft of odp_manager
Mat001 Sep 2, 2022
05b0d70
add tests
Mat001 Sep 13, 2022
e1a2de2
use unittest style of asserting exception
Mat001 Sep 13, 2022
2bb6c32
remove import
Mat001 Sep 13, 2022
32167fe
make tests more concise
Mat001 Sep 14, 2022
19492f9
fixed some pr comments
Mat001 Sep 15, 2022
baff0c4
de-indent event manager init
Mat001 Sep 15, 2022
0789c7c
add update config event manager signal
andrewleap-optimizely Sep 15, 2022
4b5989b
Merge branch 'mpirnovar/odp_manager' of github.com:optimizely/python-…
Mat001 Sep 15, 2022
4b0b338
updated tests
Mat001 Sep 16, 2022
ec96869
commit odp_manager updates and cleanup
Mat001 Sep 16, 2022
e53eaa3
addres PR comments
Mat001 Sep 20, 2022
7c7535b
refactored tests for fetch segments
Mat001 Sep 20, 2022
0a2be6d
update cache default timeout
Mat001 Sep 20, 2022
de18508
fix pypy failed test
andrewleap-optimizely Sep 21, 2022
6b7385f
hardcode cache settings
andrewleap-optimizely Sep 21, 2022
18fbf64
assert error log not called
andrewleap-optimizely Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions optimizely/helpers/enums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,9 @@ class OdpManagerConfig:
"""ODP Manager configs."""
KEY_FOR_USER_ID: Final = 'fs_user_id'
EVENT_TYPE: Final = 'fullstack'


class OdpSegmentsCacheConfig:
"""ODP Segment Cache configs."""
DEFAULT_CAPACITY: Final = 10_000
DEFAULT_TIMEOUT_SECS: Final = 10
36 changes: 22 additions & 14 deletions optimizely/odp/odp_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

from optimizely import exceptions as optimizely_exception
from optimizely import logger as optimizely_logger
from optimizely.helpers.enums import Errors, OdpManagerConfig
from optimizely.helpers.enums import Errors, OdpManagerConfig, OdpSegmentsCacheConfig
from optimizely.helpers.validator import are_odp_data_types_valid
from optimizely.odp.lru_cache import OptimizelySegmentsCache
from optimizely.odp.lru_cache import OptimizelySegmentsCache, LRUCache
from optimizely.odp.odp_config import OdpConfig, OdpConfigState
from optimizely.odp.odp_event_manager import OdpEventManager
from optimizely.odp.odp_segment_manager import OdpSegmentManager
Expand All @@ -32,7 +32,7 @@ class OdpManager:
def __init__(
self,
disable: bool,
segments_cache: OptimizelySegmentsCache,
segments_cache: Optional[OptimizelySegmentsCache] = None,
segment_manager: Optional[OdpSegmentManager] = None,
event_manager: Optional[OdpEventManager] = None,
logger: Optional[optimizely_logger.Logger] = None
Expand All @@ -42,20 +42,27 @@ def __init__(
self.odp_config = OdpConfig()
self.logger = logger or optimizely_logger.NoOpLogger()

self.segment_manager = None
self.event_manager = None
self.segment_manager = segment_manager
self.event_manager = event_manager

if not self.enabled:
self.logger.info('ODP is disabled.')
return

if segment_manager:
segment_manager.odp_config = self.odp_config
self.segment_manager = segment_manager
else:
self.segment_manager = OdpSegmentManager(self.odp_config,
segments_cache,
ZaiusGraphQLApiManager(logger), logger)
if not self.segment_manager:
if not segments_cache:
segments_cache = LRUCache(
OdpSegmentsCacheConfig.DEFAULT_CAPACITY,
OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS
)
self.segment_manager = OdpSegmentManager(
self.odp_config,
segments_cache,
ZaiusGraphQLApiManager(logger), logger
)

self.segment_manager.odp_config = self.odp_config

if event_manager:
event_manager.odp_config = self.odp_config
self.event_manager = event_manager
Expand All @@ -66,7 +73,8 @@ def __init__(

def fetch_qualified_segments(self, user_id: str, options: list[str]) -> Optional[list[str]]:
if not self.enabled or not self.segment_manager:
raise optimizely_exception.OdpNotEnabled(Errors.ODP_NOT_ENABLED)
self.logger.error(Errors.ODP_NOT_ENABLED)
return None

user_key = OdpManagerConfig.KEY_FOR_USER_ID
user_value = user_id
Expand All @@ -78,7 +86,7 @@ def identify_user(self, user_id: str) -> None:
self.logger.debug('ODP identify event is not dispatched (ODP disabled).')
return
if self.odp_config.odp_state() == OdpConfigState.NOT_INTEGRATED:
self.logger.debug('ODP Identify event is not dispatched (ODP not integrated).')
self.logger.debug('ODP identify event is not dispatched (ODP not integrated).')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 107-114, send_event raises multiple exceptions. They should be processed separately at the top level. We can discuss again when integrated.

return

self.event_manager.identify_user(user_id)
Expand Down
14 changes: 9 additions & 5 deletions optimizely/odp/odp_segment_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,21 @@
class OdpSegmentManager:
"""Schedules connections to ODP for audience segmentation and caches the results."""

def __init__(self, odp_config: OdpConfig, segments_cache: OptimizelySegmentsCache,
zaius_manager: ZaiusGraphQLApiManager,
logger: Optional[optimizely_logger.Logger] = None) -> None:
def __init__(
self,
odp_config: OdpConfig,
segments_cache: OptimizelySegmentsCache,
zaius_manager: ZaiusGraphQLApiManager,
logger: Optional[optimizely_logger.Logger] = None
) -> None:

self.odp_config = odp_config
self.segments_cache = segments_cache
self.zaius_manager = zaius_manager
self.logger = logger or optimizely_logger.NoOpLogger()

def fetch_qualified_segments(self, user_key: str, user_value: str, options: list[str]) -> \
Optional[list[str]]:
def fetch_qualified_segments(self, user_key: str, user_value: str, options: list[str]
) -> Optional[list[str]]:
"""
Args:
user_key: The key for identifying the id type.
Expand Down
44 changes: 30 additions & 14 deletions tests/test_odp_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ def test_configurations_disable_odp(self):
mock_logger = mock.MagicMock()
manager = OdpManager(True, OptimizelySegmentsCache, None, None, mock_logger)

self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED,
manager.fetch_qualified_segments, 'user1', [])

mock_logger.info.assert_called_once_with('ODP is disabled.')
manager.update_odp_config('valid', 'host', [])
self.assertIsNone(manager.odp_config.get_api_key())
self.assertIsNone(manager.odp_config.get_api_host())

manager.fetch_qualified_segments('user1', [])
mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED)

# these call should be dropped gracefully with None
manager.identify_user('user1')

Expand Down Expand Up @@ -72,21 +72,37 @@ def test_fetch_qualified_segments(self):
mock_logger.error.assert_not_called()
mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', ['IGNORE_CACHE'])

def test_fetch_qualified_segments__not_enabled(self):
def test_fetch_qualified_segments__seg_mgr_and_seg_cache_are_none(self):
mock_logger = mock.MagicMock()
segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache,
ZaiusGraphQLApiManager(mock_logger), mock_logger)

manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger)
manager.enabled = False
manager.OptimizelySegmentsCache = None
manager.segment_manager = None

with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments:
self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED,
manager.fetch_qualified_segments, 'user1', [])
manager.fetch_qualified_segments('user1', [])

# verify segment manager did not try to fetch segments
mock_logger.debug.assert_not_called()
mock_logger.error.assert_called_once_with('ODP is not enabled.')
mock_fetch_qualif_segments.assert_not_called()

def test_fetch_qualified_segments__not_enabled(self):
mock_logger = mock.MagicMock()
segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache,
ZaiusGraphQLApiManager(mock_logger), mock_logger)

manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger)
manager.enabled = False

manager.fetch_qualified_segments('user1', [])
mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED)

with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments:
# verify segment manager did not try to fetch segments
mock_fetch_qualif_segments.assert_not_called()

def test_fetch_qualified_segments__segment_mgr_not_available(self):
mock_logger = mock.MagicMock()
segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache,
Expand All @@ -95,12 +111,12 @@ def test_fetch_qualified_segments__segment_mgr_not_available(self):
manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger)
manager.segment_manager = False

with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments:
self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED,
manager.fetch_qualified_segments, 'user1', [])
manager.fetch_qualified_segments('user1', [])
mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED)

# verify segment manager did not try to fetch segments
mock_fetch_qualif_segments.assert_not_called()
with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments:
# verify segment manager did not try to fetch segments
mock_fetch_qualif_segments.assert_not_called()

def test_identify_user_datafile_not_ready(self):
mock_logger = mock.MagicMock()
Expand Down Expand Up @@ -146,7 +162,7 @@ def test_identify_user_odp_not_integrated(self):

mock_dispatch_event.assert_not_called()
mock_logger.debug.assert_any_call('Odp config was not changed.')
mock_logger.debug.assert_any_call('ODP Identify event is not dispatched (ODP not integrated).')
mock_logger.debug.assert_any_call('ODP identify event is not dispatched (ODP not integrated).')

def test_identify_user_odp_disabled(self):
mock_logger = mock.MagicMock()
Expand Down