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 2 commits
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
2 changes: 1 addition & 1 deletion optimizely/helpers/enums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,4 @@ class OdpManagerConfig:
class OdpSegmentsCacheConfig:
"""ODP Segment Cache configs."""
DEFAULT_CAPACITY: Final = 10_000
DEFAULT_TIMEOUT_SECS: Final = 10
DEFAULT_TIMEOUT_SECS: Final = 600
4 changes: 2 additions & 2 deletions optimizely/odp/odp_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ def __init__(
segments_cache,
ZaiusGraphQLApiManager(logger), logger
)

self.segment_manager.odp_config = self.odp_config
else:
self.segment_manager.odp_config = self.odp_config

if event_manager:
event_manager.odp_config = self.odp_config
Expand Down
95 changes: 49 additions & 46 deletions tests/test_odp_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from optimizely import exceptions as optimizely_exception
from optimizely import version
from optimizely.helpers.enums import Errors
from optimizely.helpers.enums import Errors, OdpSegmentsCacheConfig
from optimizely.odp.lru_cache import OptimizelySegmentsCache, LRUCache
from optimizely.odp.odp_config import OdpConfig
from optimizely.odp.odp_event_manager import OdpEventManager
Expand All @@ -32,7 +32,7 @@ class OdpManagerTest(base.BaseTest):

def test_configurations_disable_odp(self):
mock_logger = mock.MagicMock()
manager = OdpManager(True, OptimizelySegmentsCache, None, None, mock_logger)
manager = OdpManager(True, OptimizelySegmentsCache, logger=mock_logger)

mock_logger.info.assert_called_once_with('ODP is disabled.')
manager.update_odp_config('valid', 'host', [])
Expand All @@ -56,7 +56,7 @@ def test_fetch_qualified_segments(self):
segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache,
ZaiusGraphQLApiManager(mock_logger), mock_logger)

manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, logger=mock_logger)

with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments:
manager.fetch_qualified_segments('user1', [])
Expand All @@ -72,57 +72,55 @@ 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__seg_mgr_and_seg_cache_are_none(self):
def test_fetch_qualified_segments__disabled(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.OptimizelySegmentsCache = None
manager.segment_manager = None
manager = OdpManager(True, OptimizelySegmentsCache, segment_manager, logger=mock_logger)

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

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):
def test_fetch_qualified_segments__segment_mgr_is_none(self):
"""
When segment manager is None, then fetching segment
should take place using the default segment manager.
"""
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 = OdpManager(False, LRUCache(10, 20), logger=mock_logger)
manager.update_odp_config('api_key', 'api_host', [])

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

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()
mock_logger.debug.assert_not_called()
mock_logger.error.assert_not_called()
mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', [])

def test_fetch_qualified_segments__segment_mgr_not_available(self):
def test_fetch_qualified_segments__seg_cache_and_seg_mgr_are_none(self):
"""
When segment cache and segment manager are None, then fetching segment
should take place using the default managers.
"""
mock_logger = mock.MagicMock()
segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache,
ZaiusGraphQLApiManager(mock_logger), mock_logger)

manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger)
manager.segment_manager = False
manager = OdpManager(False, mock_logger)
manager.update_odp_config('api_key', 'api_host', [])

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

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()
mock_logger.debug.assert_not_called()
mock_logger.error.assert_not_called()
mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', [])

def test_identify_user_datafile_not_ready(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger)

manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)

with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
manager.identify_user('user1')
Expand All @@ -133,7 +131,7 @@ def test_identify_user_odp_integrated(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger)
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -154,7 +152,7 @@ def test_identify_user_odp_not_integrated(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)
manager.update_odp_config(None, None, [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -168,7 +166,7 @@ def test_identify_user_odp_disabled(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

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

with mock.patch.object(event_manager, 'identify_user') as mock_identify_user:
Expand All @@ -181,7 +179,7 @@ def test_send_event_datafile_not_ready(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'})
Expand All @@ -193,7 +191,7 @@ def test_send_event_odp_integrated(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger)
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -215,7 +213,7 @@ def test_send_event_odp_not_integrated(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)
manager.update_odp_config(None, None, [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -229,7 +227,7 @@ def test_send_event_odp_disabled(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

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

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -243,7 +241,7 @@ def test_send_event_odp_disabled__event_manager_not_available(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)
manager.event_manager = False

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -257,7 +255,7 @@ def test_send_event_invalid_data(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger)
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', [])

with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event:
Expand All @@ -270,7 +268,7 @@ def test_config_not_changed(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())

manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger)
manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger)
manager.update_odp_config(None, None, [])
mock_logger.debug.assert_called_with('Odp config was not changed.')

Expand Down Expand Up @@ -325,7 +323,7 @@ def test_update_odp_config__update_config_called(self):
"""
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())
manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger)
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)

with mock.patch.object(event_manager, 'update_config') as mock_update:
first_api_key = manager.odp_config.get_api_key()
Expand Down Expand Up @@ -361,7 +359,7 @@ def test_update_odp_config__update_config_called(self):
def test_update_odp_config__odp_config_propagated_properly(self):
mock_logger = mock.MagicMock()
event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager())
manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger)
manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger)
manager.update_odp_config('key1', 'host1', ['a', 'b'])

self.assertEqual(manager.segment_manager.odp_config.get_api_key(), 'key1')
Expand All @@ -384,3 +382,8 @@ def test_update_odp_config__odp_config_propagated_properly(self):
manager.update_odp_config(None, None, ['a', 'b'])
self.assertEqual(manager.segment_manager.odp_config.get_segments_to_check(), ['a', 'b'])
self.assertEqual(manager.event_manager.odp_config.get_segments_to_check(), ['a', 'b'])

def test_segments_cache_default_settings(self):
manager = OdpManager(False)
self.assertEqual(manager.segment_manager.segments_cache.capacity, OdpSegmentsCacheConfig.DEFAULT_CAPACITY)
self.assertEqual(manager.segment_manager.segments_cache.timeout, OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS)