feat: add odp manager #314
Conversation
lib/optimizely/odp/odp_manager.rb
Outdated
| # | ||
| # @return - Array of qualified segments or nil. | ||
| options ||= [] | ||
| unless @enabled && @segment_manager |
There was a problem hiding this comment.
| unless @enabled && @segment_manager | |
| unless @enabled |
segment_manager is not-null always?
There was a problem hiding this comment.
True, I'm just used to convincing the type checker... but ruby does not care, so it's removed 🥳
lib/optimizely/odp/odp_manager.rb
Outdated
| end | ||
|
|
||
| unless Helpers::Validator.odp_data_types_valid?(data) | ||
| @logger.log(Logger::DEBUG, format(ODP_LOGS[:ODP_EVENT_NOT_DISPATCHED], action, 'ODP data is not valid')) |
There was a problem hiding this comment.
| @logger.log(Logger::DEBUG, format(ODP_LOGS[:ODP_EVENT_NOT_DISPATCHED], action, 'ODP data is not valid')) | |
| @logger.log(Logger::ERROR, format(ODP_LOGS[:ODP_EVENT_NOT_DISPATCHED], action, ODP_NOT_ENABLED)) |
lib/optimizely/odp/odp_manager.rb
Outdated
| # @param identifiers - a hash for identifiers. | ||
| # @param data - a hash for associated data. The default event data will be added to this data before sending to the ODP server. | ||
| unless @enabled && @event_manager | ||
| @logger.log(Logger::DEBUG, format(ODP_LOGS[:ODP_EVENT_NOT_DISPATCHED], action, 'ODP disabled')) |
There was a problem hiding this comment.
| @logger.log(Logger::DEBUG, format(ODP_LOGS[:ODP_EVENT_NOT_DISPATCHED], action, 'ODP disabled')) | |
| @logger.log(Logger::DEBUG, format(ODP_LOGS[:ODP_EVENT_NOT_DISPATCHED], action, ODP_NOT_ENABLED)) |
lib/optimizely/odp/odp_manager.rb
Outdated
| end | ||
|
|
||
| def close! | ||
| @event_manager&.stop! |
There was a problem hiding this comment.
| @event_manager&.stop! | |
| return unless @enabled | |
| @event_manager.stop! |
Just for clarity?
lib/optimizely/odp/odp_manager.rb
Outdated
| @odp_config = OdpConfig.new | ||
|
|
||
| unless @enabled | ||
| @logger.log(Logger::DEBUG, 'ODP is disabled') |
There was a problem hiding this comment.
| @logger.log(Logger::DEBUG, 'ODP is disabled') | |
| @logger.log(Logger::INFO, ODP_NOT_ENABLED) |
lib/optimizely/odp/odp_manager.rb
Outdated
| end | ||
|
|
||
| def identify_user(user_id:) | ||
| send_event( |
There was a problem hiding this comment.
Can we call "@event_manager.send_event()" directly?
"identify_user" is implicit call by SDK while "send_event" explicit call by the client, so errors can be handled differently. When ODP_DISABLED, "identify_user" can drop silently (with debug log) and can skip parameter validation, while "send_event" should log error.
There was a problem hiding this comment.
Makes sense to me
| @event_manager.send_event(type: type, action: action, identifiers: identifiers, data: data) | ||
| end | ||
|
|
||
| def update_odp_config(api_key, api_host, segments_to_check) |
There was a problem hiding this comment.
This is great! I am just curios who will call this function? should we add a notification listener here and make odpmanager self update itself? Or are we expecting to do something like that and trigger updates from the outside by calling this method?
There was a problem hiding this comment.
Yea the idea is to register the listener at client initialization and have the callback be a client method, thus avoiding having to pass in the NotificationListener and the ProjectConfig. Seems to make sense because the callback needs access to both ProjectConfig and OdpConfig. Thoughts/opinions?
Summary
Provides all internal services related to the ODP integration:
Internal API:
Test plan
Ticket