Skip to content

feat: odp segment manager #402

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

Merged
merged 10 commits into from
Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
feat: add segment manager
  • Loading branch information
Mat001 committed Aug 23, 2022
commit 80659967e69f6980bc958013d1f137cefff28075
4 changes: 2 additions & 2 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,5 +209,5 @@ class OdpGraphQLApiConfig:

class OptimizelySegmentOption:
"""Options for the OdpSegmentManager."""
IGNORE_CACHE = None
RESET_CACHE = None
IGNORE_CACHE = 'IGNORE_CACHE'
RESET_CACHE = 'RESET_CACHE'
41 changes: 12 additions & 29 deletions optimizely/odp/odp_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def __init__(
api_host: Optional[str] = None,
segments_to_check: Optional[list[str]] = None
) -> None:
self.api_key = api_key
self.api_host = api_host
self.segments_to_check = segments_to_check or []
self._api_key = api_key
self._api_host = api_host
self._segments_to_check = segments_to_check or []
self.lock = Lock()

def update(self, api_key: Optional[str], api_host: Optional[str], segments_to_check: list[str]) -> bool:
Expand All @@ -53,44 +53,27 @@ def update(self, api_key: Optional[str], api_host: Optional[str], segments_to_ch
"""
updated = False
with self.lock:
if self.api_key != api_key or self.api_host != api_host or self.segments_to_check != segments_to_check:
self.api_key = api_key
self.api_host = api_host
self.segments_to_check = segments_to_check
if self._api_key != api_key or self._api_host != api_host or self._segments_to_check != segments_to_check:
self._api_key = api_key
self._api_host = api_host
self._segments_to_check = segments_to_check
updated = True

return updated

def get_api_host(self) -> Optional[str]:
with self.lock:
return self.api_host

def set_api_host(self, api_host: Optional[str]) -> None:
with self.lock:
self.api_host = api_host
return self._api_host

def get_api_key(self) -> Optional[str]:
with self.lock:
return self.api_key
return self._api_key

def set_api_key(self, api_key: Optional[str]) -> None:
with self.lock:
self.api_key = api_key

def get_segments_to_check(self) -> Optional[list[str]]:
with self.lock:
return self.segments_to_check.copy()

def set_segments_to_check(self, segments_to_check: list[str]) -> None:
"""
Replace qualified segments with provided list of segments.
Args:
segments_to_check: A list of segment names.
"""
def get_segments_to_check(self) -> list[str]:
with self.lock:
self.segments_to_check = segments_to_check.copy()
return self._segments_to_check.copy()

def odp_integrated(self) -> bool:
"""Returns True if ODP is integrated."""
with self.lock:
return self.api_key is not None and self.api_host is not None
return self._api_key is not None and self._api_host is not None
12 changes: 6 additions & 6 deletions optimizely/odp/odp_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@

from __future__ import annotations

from typing import Any, Dict
from typing import Any


class OdpEvent:
""" Representation of an odp event which can be sent to the Optimizely odp platform. """

def __init__(self, type: str, action: str,
identifiers: Dict[str, str], data: Dict[str, Any]) -> None:
self.type = type,
self.action = action,
self.identifiers = identifiers,
self.data = data
identifiers: dict[str, str], data: dict[str, Any]) -> None:
self.type = type
self.action = action
self.identifiers = identifiers
self.data = data
82 changes: 82 additions & 0 deletions optimizely/odp/odp_segment_manager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Copyright 2022, Optimizely
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

from typing import List, Optional

from optimizely import logger as optimizely_logger
from optimizely.helpers.enums import Errors
from optimizely.helpers.enums import OptimizelySegmentOption
from optimizely.odp.lru_cache import LRUCache
from optimizely.odp.odp_config import OdpConfig
from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager


class OdpSegmentManager:
"""Schedules connections to ODP for audience segmentation and caches the results."""

IGNORE_CACHE = OptimizelySegmentOption.IGNORE_CACHE
RESET_CACHE = OptimizelySegmentOption.RESET_CACHE

def __init__(self, odp_config: Optional[OdpConfig], segments_cache: Optional[LRUCache[str, List[str]](1000, 1000)],
zaius_manager: Optional[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[OptimizelySegmentOption]):
if not self.odp_config.odp_integrated():
self.logger.error(Errors.FETCH_SEGMENTS_FAILED.format('apiKey/apiHost not defined'))
return None

odp_api_key: Optional[str] = self.odp_config.get_api_key()
odp_api_host: Optional[str] = self.odp_config.get_api_host()
odp_segments_to_check: Optional[list[str]] = self.odp_config.get_segments_to_check()

if not odp_segments_to_check and not len(odp_segments_to_check):
self.logger.debug('No segments are used in the project. Returning empty list.')
return []

cache_key = self.make_cache_key(user_key, user_value)

ignore_cache = self.IGNORE_CACHE if self.IGNORE_CACHE in options else None
reset_cache = self.RESET_CACHE if self.RESET_CACHE in options else None

if reset_cache:
self._reset()

if not ignore_cache and not reset_cache:
segments = self.segments_cache.lookup(cache_key)
if segments:
self.logger.debug('ODP cache hit. Returning segments from cache.')
return segments

self.logger.debug('ODP cache miss. Making a call to ODP server.')
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessarily a cache miss... We might want to split this into two debug logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewleap-optimizely Not sure what you mean, can you clarify? If we don't hit the cache we miss it no?
U mean if we don't hit the cache there is the third option that is not a cache miss?

Rb segment manager has the same btw

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean that the cache might not have been checked, because ignore_cache was set.

Yea I should change it in ruby and probably Java where I pulled it from 😀

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like this:

if not ignore_cache and not reset_cache:
    segments = self.segments_cache.lookup(cache_key)
    if segments:
        self.logger.debug('ODP cache hit. Returning segments from cache.')
        return segments
    self.logger.debug('ODP cache miss.')

self.logger.debug('Making a call to ODP server.')

Otherwise looking at the logs with a disabled cache may confuse the reader into thinking the cache is being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, yeah, that makes total sense. good catch


segments = self.zaius_manager.fetch_segments(odp_api_key, odp_api_host, user_key, user_value,
odp_segments_to_check)

if segments and not ignore_cache:
self.segments_cache.save(cache_key, segments)

return segments

def _reset(self):
self.segments_cache.reset()

def make_cache_key(self, user_key: str, user_value: str) -> str:
return user_key + '-$-' + user_value
41 changes: 41 additions & 0 deletions tests/test_odp_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Copyright 2022, Optimizely
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http:#www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations
from tests import base
from optimizely.odp.odp_config import OdpConfig


class OdpConfigTest(base.BaseTest):
api_host = 'test-host'
api_key = 'test-key'
segments_to_check = ['test-segment']

def test_init_config(self):
config = OdpConfig(self.api_key, self.api_host, self.segments_to_check)

self.assertEqual(config.get_api_key(), self.api_key)
self.assertEqual(config.get_api_host(), self.api_host)
self.assertEqual(config.get_segments_to_check(), self.segments_to_check)

def test_update_config(self):
config = OdpConfig()
updated = config.update(self.api_key, self.api_host, self.segments_to_check)

self.assertStrictTrue(updated)
self.assertEqual(config.get_api_key(), self.api_key)
self.assertEqual(config.get_api_host(), self.api_host)
self.assertEqual(config.get_segments_to_check(), self.segments_to_check)

updated = config.update(self.api_key, self.api_host, self.segments_to_check)
self.assertStrictFalse(updated)
Loading