Skip to content

Conversation

@andrewleap-optimizely
Copy link
Contributor

Summary

Adds OdpSegmentManager to provide internal services for fetching segments from the ODP server.

  • Provides an internal api for fetching segments with given { api_key, api_host, customer key, customer value, segments subset, and options }.
  • Manages a LRU cache for fetched segments (size and timeout are configurable).
  • On segment fetch request, looks up the cache for the user key first. On cache hit (or timeout not expired) it returns the cached segments. Otherwise, it sends a fetch request to the ODP server and caches the result.
  • Supports options for cache lookup (IGNORE_CACHE, RESET_CACHE).

Test plan

  • Added odp_segment_manager_spec.rb

Ticket

OASIS-8446

Copy link
Contributor

@zashraf1985 zashraf1985 left a comment

Choose a reason for hiding this comment

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

Looks Great! Suggested a few changes.

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

Looks good. A few small changes suggested.
Some ruby contexts I cannot cover :)

@api_key = api_key
@api_host = api_host
@segments_to_check = segments_to_check
@odp_enabled = !api_key.nil? && !api_host.nil? ? true : false
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll have a similar flag (odpEnabled) in the OdpManager level, which disable all ODP functions including segments and events. This may be conflict with that. We can clean it up here and visit again at the top level.

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zashraf1985 zashraf1985 left a comment

Choose a reason for hiding this comment

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

Looks Great!

@andrewleap-optimizely andrewleap-optimizely merged commit cfc1efa into master Aug 19, 2022
@andrewleap-optimizely andrewleap-optimizely deleted the aleap/add_odp_segment_manager branch August 19, 2022 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants