Skip to content

Commit 92ab102

Browse files
andrewleap-optimizelyandrewleap-optimizely
authored
refactor: remove odp config from constructors (#406)
* remove odp_config from event_manager init * remove odp_config from segment_manager init
1 parent 082f171 commit 92ab102

File tree

6 files changed

+109
-96
lines changed

6 files changed

+109
-96
lines changed

‎optimizely/odp/odp_event_manager.py‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,23 +44,21 @@ class OdpEventManager:
4444

4545
def __init__(
4646
self,
47-
odp_config: OdpConfig,
4847
logger: Optional[_logging.Logger] = None,
4948
api_manager: Optional[ZaiusRestApiManager] = None
5049
):
5150
"""OdpEventManager init method to configure event batching.
5251
5352
Args:
54-
odp_config: ODP integration config.
5553
logger: Optional component which provides a log method to log messages. By default nothing would be logged.
5654
api_manager: Optional component which sends events to ODP.
5755
"""
5856
self.logger = logger or _logging.NoOpLogger()
5957
self.zaius_manager = api_manager or ZaiusRestApiManager(self.logger)
6058

61-
self.odp_config = odp_config
62-
self.api_key = odp_config.get_api_key()
63-
self.api_host = odp_config.get_api_host()
59+
self.odp_config: Optional[OdpConfig] = None
60+
self.api_key: Optional[str] = None
61+
self.api_host: Optional[str] = None
6462

6563
self.event_queue: Queue[OdpEvent | Signal] = Queue(OdpEventManagerConfig.DEFAULT_QUEUE_CAPACITY)
6664
self.batch_size = OdpEventManagerConfig.DEFAULT_BATCH_SIZE
@@ -78,12 +76,16 @@ def is_running(self) -> bool:
7876
"""Property to check if consumer thread is alive or not."""
7977
return self.thread.is_alive()
8078

81-
def start(self) -> None:
79+
def start(self, odp_config: OdpConfig) -> None:
8280
"""Starts the batch processing thread to batch events."""
8381
if self.is_running:
8482
self.logger.warning('ODP event queue already started.')
8583
return
8684

85+
self.odp_config = odp_config
86+
self.api_host = self.odp_config.get_api_host()
87+
self.api_key = self.odp_config.get_api_key()
88+
8789
self.thread.start()
8890

8991
def _run(self) -> None:
@@ -217,6 +219,10 @@ def stop(self) -> None:
217219

218220
def send_event(self, type: str, action: str, identifiers: dict[str, str], data: OdpDataDict) -> None:
219221
"""Create OdpEvent and add it to the event queue."""
222+
if not self.odp_config:
223+
self.logger.debug('ODP event queue: cannot send before config has been set.')
224+
return
225+
220226
odp_state = self.odp_config.odp_state()
221227
if odp_state == OdpConfigState.UNDETERMINED:
222228
self.logger.debug('ODP event queue: cannot send before the datafile has loaded.')
@@ -260,5 +266,6 @@ def _update_config(self) -> None:
260266
if len(self._current_batch) > 0:
261267
self._flush_batch()
262268

263-
self.api_host = self.odp_config.get_api_host()
264-
self.api_key = self.odp_config.get_api_key()
269+
if self.odp_config:
270+
self.api_host = self.odp_config.get_api_host()
271+
self.api_key = self.odp_config.get_api_key()

‎optimizely/odp/odp_manager.py‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
from optimizely.odp.odp_config import OdpConfig, OdpConfigState
2424
from optimizely.odp.odp_event_manager import OdpEventManager
2525
from optimizely.odp.odp_segment_manager import OdpSegmentManager
26-
from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager
2726

2827

2928
class OdpManager:
@@ -55,21 +54,15 @@ def __init__(
5554
OdpSegmentsCacheConfig.DEFAULT_CAPACITY,
5655
OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS
5756
)
58-
self.segment_manager = OdpSegmentManager(
59-
self.odp_config,
60-
segments_cache,
61-
ZaiusGraphQLApiManager(logger), logger
62-
)
63-
else:
64-
self.segment_manager.odp_config = self.odp_config
57+
self.segment_manager = OdpSegmentManager(segments_cache, logger=self.logger)
6558

6659
if event_manager:
67-
event_manager.odp_config = self.odp_config
6860
self.event_manager = event_manager
6961
else:
70-
self.event_manager = OdpEventManager(self.odp_config, logger)
62+
self.event_manager = OdpEventManager(self.logger)
7163

72-
self.event_manager.start()
64+
self.segment_manager.odp_config = self.odp_config
65+
self.event_manager.start(self.odp_config)
7366

7467
def fetch_qualified_segments(self, user_id: str, options: list[str]) -> Optional[list[str]]:
7568
if not self.enabled or not self.segment_manager:

‎optimizely/odp/odp_segment_manager.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717

1818
from optimizely import logger as optimizely_logger
1919
from optimizely.helpers.enums import Errors
20+
from optimizely.odp.odp_config import OdpConfig
2021
from optimizely.odp.optimizely_odp_option import OptimizelyOdpOption
2122
from optimizely.odp.lru_cache import OptimizelySegmentsCache
22-
from optimizely.odp.odp_config import OdpConfig
2323
from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager
2424

2525

@@ -28,16 +28,15 @@ class OdpSegmentManager:
2828

2929
def __init__(
3030
self,
31-
odp_config: OdpConfig,
3231
segments_cache: OptimizelySegmentsCache,
33-
zaius_manager: ZaiusGraphQLApiManager,
32+
zaius_manager: Optional[ZaiusGraphQLApiManager] = None,
3433
logger: Optional[optimizely_logger.Logger] = None
3534
) -> None:
3635

37-
self.odp_config = odp_config
36+
self.odp_config: Optional[OdpConfig] = None
3837
self.segments_cache = segments_cache
39-
self.zaius_manager = zaius_manager
4038
self.logger = logger or optimizely_logger.NoOpLogger()
39+
self.zaius_manager = zaius_manager or ZaiusGraphQLApiManager(self.logger)
4140

4241
def fetch_qualified_segments(self, user_key: str, user_value: str, options: list[str]
4342
) -> Optional[list[str]]:
@@ -50,11 +49,12 @@ def fetch_qualified_segments(self, user_key: str, user_value: str, options: list
5049
Returns:
5150
Qualified segments for the user from the cache or the ODP server if not in the cache.
5251
"""
53-
odp_api_key = self.odp_config.get_api_key()
54-
odp_api_host = self.odp_config.get_api_host()
55-
odp_segments_to_check = self.odp_config.get_segments_to_check()
52+
if self.odp_config:
53+
odp_api_key = self.odp_config.get_api_key()
54+
odp_api_host = self.odp_config.get_api_host()
55+
odp_segments_to_check = self.odp_config.get_segments_to_check()
5656

57-
if not (odp_api_key and odp_api_host):
57+
if not self.odp_config or not (odp_api_key and odp_api_host):
5858
self.logger.error(Errors.FETCH_SEGMENTS_FAILED.format('api_key/api_host not defined'))
5959
return None
6060

‎tests/test_odp_event_manager.py‎

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ def test_invalid_odp_event(self, *args):
100100

101101
def test_odp_event_manager_success(self, *args):
102102
mock_logger = mock.Mock()
103-
event_manager = OdpEventManager(self.odp_config, mock_logger)
104-
event_manager.start()
103+
event_manager = OdpEventManager(mock_logger)
104+
event_manager.start(self.odp_config)
105105

106106
with mock.patch('requests.post', return_value=self.fake_server_response(status_code=200)):
107107
event_manager.send_event(**self.events[0])
@@ -116,8 +116,8 @@ def test_odp_event_manager_success(self, *args):
116116

117117
def test_odp_event_manager_batch(self, *args):
118118
mock_logger = mock.Mock()
119-
event_manager = OdpEventManager(self.odp_config, mock_logger)
120-
event_manager.start()
119+
event_manager = OdpEventManager(mock_logger)
120+
event_manager.start(self.odp_config)
121121

122122
event_manager.batch_size = 2
123123
with mock.patch.object(
@@ -135,8 +135,8 @@ def test_odp_event_manager_batch(self, *args):
135135

136136
def test_odp_event_manager_multiple_batches(self, *args):
137137
mock_logger = mock.Mock()
138-
event_manager = OdpEventManager(self.odp_config, mock_logger)
139-
event_manager.start()
138+
event_manager = OdpEventManager(mock_logger)
139+
event_manager.start(self.odp_config)
140140

141141
event_manager.batch_size = 2
142142
batch_count = 4
@@ -164,7 +164,8 @@ def test_odp_event_manager_multiple_batches(self, *args):
164164

165165
def test_odp_event_manager_backlog(self, *args):
166166
mock_logger = mock.Mock()
167-
event_manager = OdpEventManager(self.odp_config, mock_logger)
167+
event_manager = OdpEventManager(mock_logger)
168+
event_manager.odp_config = self.odp_config
168169

169170
event_manager.batch_size = 2
170171
batch_count = 4
@@ -178,7 +179,7 @@ def test_odp_event_manager_backlog(self, *args):
178179
with mock.patch.object(
179180
event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, return_value=False
180181
) as mock_send:
181-
event_manager.start()
182+
event_manager.start(self.odp_config)
182183
event_manager.send_event(**self.events[0])
183184
event_manager.send_event(**self.events[1])
184185
event_manager.stop()
@@ -198,8 +199,8 @@ def test_odp_event_manager_backlog(self, *args):
198199

199200
def test_odp_event_manager_flush(self, *args):
200201
mock_logger = mock.Mock()
201-
event_manager = OdpEventManager(self.odp_config, mock_logger)
202-
event_manager.start()
202+
event_manager = OdpEventManager(mock_logger)
203+
event_manager.start(self.odp_config)
203204

204205
with mock.patch.object(
205206
event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, return_value=False
@@ -217,8 +218,8 @@ def test_odp_event_manager_flush(self, *args):
217218

218219
def test_odp_event_manager_multiple_flushes(self, *args):
219220
mock_logger = mock.Mock()
220-
event_manager = OdpEventManager(self.odp_config, mock_logger)
221-
event_manager.start()
221+
event_manager = OdpEventManager(mock_logger)
222+
event_manager.start(self.odp_config)
222223
flush_count = 4
223224

224225
with mock.patch.object(
@@ -244,8 +245,8 @@ def test_odp_event_manager_multiple_flushes(self, *args):
244245

245246
def test_odp_event_manager_retry_failure(self, *args):
246247
mock_logger = mock.Mock()
247-
event_manager = OdpEventManager(self.odp_config, mock_logger)
248-
event_manager.start()
248+
event_manager = OdpEventManager(mock_logger)
249+
event_manager.start(self.odp_config)
249250

250251
number_of_tries = event_manager.retry_count + 1
251252

@@ -269,8 +270,8 @@ def test_odp_event_manager_retry_failure(self, *args):
269270

270271
def test_odp_event_manager_retry_success(self, *args):
271272
mock_logger = mock.Mock()
272-
event_manager = OdpEventManager(self.odp_config, mock_logger)
273-
event_manager.start()
273+
event_manager = OdpEventManager(mock_logger)
274+
event_manager.start(self.odp_config)
274275

275276
with mock.patch.object(
276277
event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, side_effect=[True, True, False]
@@ -289,8 +290,8 @@ def test_odp_event_manager_retry_success(self, *args):
289290

290291
def test_odp_event_manager_send_failure(self, *args):
291292
mock_logger = mock.Mock()
292-
event_manager = OdpEventManager(self.odp_config, mock_logger)
293-
event_manager.start()
293+
event_manager = OdpEventManager(mock_logger)
294+
event_manager.start(self.odp_config)
294295

295296
with mock.patch.object(
296297
event_manager.zaius_manager,
@@ -313,8 +314,8 @@ def test_odp_event_manager_disabled(self, *args):
313314
mock_logger = mock.Mock()
314315
odp_config = OdpConfig()
315316
odp_config.update(None, None, None)
316-
event_manager = OdpEventManager(odp_config, mock_logger)
317-
event_manager.start()
317+
event_manager = OdpEventManager(mock_logger)
318+
event_manager.start(odp_config)
318319

319320
event_manager.send_event(**self.events[0])
320321
event_manager.send_event(**self.events[1])
@@ -330,7 +331,9 @@ def test_odp_event_manager_queue_full(self, *args):
330331
mock_logger = mock.Mock()
331332

332333
with mock.patch('optimizely.helpers.enums.OdpEventManagerConfig.DEFAULT_QUEUE_CAPACITY', 1):
333-
event_manager = OdpEventManager(self.odp_config, mock_logger)
334+
event_manager = OdpEventManager(mock_logger)
335+
336+
event_manager.odp_config = self.odp_config
334337

335338
with mock.patch('optimizely.odp.odp_event_manager.OdpEventManager.is_running', True):
336339
event_manager.send_event(**self.events[0])
@@ -344,8 +347,8 @@ def test_odp_event_manager_queue_full(self, *args):
344347

345348
def test_odp_event_manager_thread_exception(self, *args):
346349
mock_logger = mock.Mock()
347-
event_manager = MockOdpEventManager(self.odp_config, mock_logger)
348-
event_manager.start()
350+
event_manager = MockOdpEventManager(mock_logger)
351+
event_manager.start(self.odp_config)
349352

350353
event_manager.send_event(**self.events[0])
351354
time.sleep(.1)
@@ -360,8 +363,8 @@ def test_odp_event_manager_thread_exception(self, *args):
360363

361364
def test_odp_event_manager_override_default_data(self, *args):
362365
mock_logger = mock.Mock()
363-
event_manager = OdpEventManager(self.odp_config, mock_logger)
364-
event_manager.start()
366+
event_manager = OdpEventManager(mock_logger)
367+
event_manager.start(self.odp_config)
365368

366369
event = deepcopy(self.events[0])
367370
event['data']['data_source'] = 'my-app'
@@ -381,9 +384,9 @@ def test_odp_event_manager_override_default_data(self, *args):
381384

382385
def test_odp_event_manager_flush_timeout(self, *args):
383386
mock_logger = mock.Mock()
384-
event_manager = OdpEventManager(self.odp_config, mock_logger)
387+
event_manager = OdpEventManager(mock_logger)
385388
event_manager.flush_interval = .5
386-
event_manager.start()
389+
event_manager.start(self.odp_config)
387390

388391
with mock.patch.object(
389392
event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, return_value=False
@@ -401,8 +404,8 @@ def test_odp_event_manager_flush_timeout(self, *args):
401404
def test_odp_event_manager_events_before_odp_ready(self, *args):
402405
mock_logger = mock.Mock()
403406
odp_config = OdpConfig()
404-
event_manager = OdpEventManager(odp_config, mock_logger)
405-
event_manager.start()
407+
event_manager = OdpEventManager(mock_logger)
408+
event_manager.start(odp_config)
406409

407410
with mock.patch.object(
408411
event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, return_value=False
@@ -436,8 +439,8 @@ def test_odp_event_manager_events_before_odp_ready(self, *args):
436439
def test_odp_event_manager_events_before_odp_disabled(self, *args):
437440
mock_logger = mock.Mock()
438441
odp_config = OdpConfig()
439-
event_manager = OdpEventManager(odp_config, mock_logger)
440-
event_manager.start()
442+
event_manager = OdpEventManager(mock_logger)
443+
event_manager.start(odp_config)
441444

442445
with mock.patch.object(event_manager.zaius_manager, 'send_odp_events') as mock_send:
443446
event_manager.send_event(**self.events[0])
@@ -467,8 +470,8 @@ def test_odp_event_manager_events_before_odp_disabled(self, *args):
467470
def test_odp_event_manager_disabled_after_init(self, *args):
468471
mock_logger = mock.Mock()
469472
odp_config = OdpConfig(self.api_key, self.api_host)
470-
event_manager = OdpEventManager(odp_config, mock_logger)
471-
event_manager.start()
473+
event_manager = OdpEventManager(mock_logger)
474+
event_manager.start(odp_config)
472475
event_manager.batch_size = 2
473476

474477
with mock.patch.object(
@@ -499,19 +502,20 @@ def test_odp_event_manager_disabled_after_events_in_queue(self, *args):
499502
mock_logger = mock.Mock()
500503
odp_config = OdpConfig(self.api_key, self.api_host)
501504

502-
event_manager = OdpEventManager(odp_config, mock_logger)
505+
event_manager = OdpEventManager(mock_logger)
506+
event_manager.odp_config = odp_config
503507
event_manager.batch_size = 3
504508

505509
with mock.patch('optimizely.odp.odp_event_manager.OdpEventManager.is_running', True):
506510
event_manager.send_event(**self.events[0])
507511
event_manager.send_event(**self.events[1])
508-
odp_config.update(None, None, [])
509-
event_manager.update_config()
510512

511513
with mock.patch.object(
512514
event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, return_value=False
513515
) as mock_send:
514-
event_manager.start()
516+
event_manager.start(odp_config)
517+
odp_config.update(None, None, [])
518+
event_manager.update_config()
515519
event_manager.send_event(**self.events[0])
516520
event_manager.send_event(**self.events[1])
517521
event_manager.send_event(**self.events[0])
@@ -522,3 +526,10 @@ def test_odp_event_manager_disabled_after_events_in_queue(self, *args):
522526
mock_logger.error.assert_not_called()
523527
mock_send.assert_called_once_with(self.api_key, self.api_host, self.processed_events)
524528
event_manager.stop()
529+
530+
def test_send_event_before_config_set(self, *args):
531+
mock_logger = mock.Mock()
532+
533+
event_manager = OdpEventManager(mock_logger)
534+
event_manager.send_event(**self.events[0])
535+
mock_logger.debug.assert_called_with('ODP event queue: cannot send before config has been set.')

0 commit comments

Comments
 (0)