feat: add odp manager #405
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewleap-optimizely
requested changes
Sep 14, 2022
jaeopt
requested changes
Sep 15, 2022
andrewleap-optimizely
requested changes
Sep 19, 2022
jaeopt
requested changes
Sep 19, 2022
optimizely/odp/odp_manager.py
Outdated
| self.odp_config = OdpConfig() | ||
| self.logger = logger or optimizely_logger.NoOpLogger() | ||
|
|
||
| self.segment_manager = None | ||
| self.event_manager = None | ||
|
|
||
| if not self.enabled: | ||
| self.logger.info('ODP is disabled.') | ||
| return | ||
|
|
||
| if segment_manager: | ||
| segment_manager.odp_config = self.odp_config | ||
| self.segment_manager = segment_manager | ||
| else: | ||
| self.segment_manager = OdpSegmentManager(self.odp_config, |
There was a problem hiding this comment.
Can we consider removing odp_config from the constructor params? It should be overriden for custom segment-manager anyway? Let's talk about it when integrating at the top level.
Mat001
commented
Sep 20, 2022
jaeopt
requested changes
Sep 20, 2022
| @@ -78,7 +86,7 @@ def identify_user(self, user_id: str) -> None: | |||
| self.logger.debug('ODP identify event is not dispatched (ODP disabled).') | |||
| return | |||
| if self.odp_config.odp_state() == OdpConfigState.NOT_INTEGRATED: | |||
| self.logger.debug('ODP Identify event is not dispatched (ODP not integrated).') | |||
| self.logger.debug('ODP identify event is not dispatched (ODP not integrated).') | |||
There was a problem hiding this comment.
Line 107-114, send_event raises multiple exceptions. They should be processed separately at the top level. We can discuss again when integrated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Issues