[FSSDK-10763] Implement UPS request batching for decideForKeys #440
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
…file` in decision logic. 🛠️ optimizely/user_profile.py -> Improved user profile loading with missing key checks. 🛠️ tests/test_decision_service.py -> Updated tests to include user profile tracker.
🛠️ tests/test_decision_service.py -> Updated experiment bucket map call. 🛠️ tests/test_decision_service.py -> Introduced user_profile_tracker usage. 🛠️ tests/test_decision_service.py -> Modified method calls with user_profile_tracker.
tests/test_decision_service.py -> Added user profile tracker object
tests/test_decision_service.py -> Fixed tests
Mat001
requested changes
Nov 20, 2024
raju-opti
requested changes
Nov 21, 2024
optimizely/decision_service.py
Outdated
| self.user_profile_service.save(user_profile.__dict__) | ||
| user_profile_tracker.update_user_profile(experiment, variation) | ||
| if self.user_profile_service is not None: | ||
| self.user_profile_service.save(user_profile_tracker.get_user_profile().__dict__) |
There was a problem hiding this comment.
This will cause the user profile service to be saved for each flag key, making the batching save ineffective. This if block should be removed
| @@ -629,8 +632,11 @@ def get_variation( | |||
| return None | |||
|
|
|||
| user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) | |||
|
|
|||
| variation, _ = self.decision_service.get_variation(project_config, experiment, user_context) | |||
| user_profile_tracker = user_profile.UserProfileTracker(user_id, self.user_profile_service, self.logger) | |||
There was a problem hiding this comment.
should there be a user_profile_service.load_user_profile() call here. I suspect this could be a reason behind FSC failures
| variation, _ = self.decision_service.get_variation(project_config, | ||
| experiment, | ||
| user_context, | ||
| user_profile_tracker) |
There was a problem hiding this comment.
the user profile should be saved here as we will remove the save from get_variation() in decision service.
optimizely/optimizely.py -> Added loading and saving profile logic
optimizely/user_profile.py -> Testing user profile update logic
optimizely/user_profile.py -> Removed unused import statement
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