Skip to content
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

fix: block odp methods on datafile #419

Merged
merged 5 commits into from
Mar 20, 2023

Conversation

andrewleap-optimizely
Copy link
Contributor

Summary

  • Added blocking call to Optimizely._identify, Optimizely._fetch_qualified_segments and Optimizely.send_odp_event to ensure datafile has been downloaded before attempting to make odp requests.

Test plan

all existing tests pass

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 - a clarification

Comment on lines 1360 to 1362
if not self.odp_manager.enabled:
self.logger.debug('ODP identify event is not dispatched (ODP disabled).')
return
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is checked in odp_manager. Do we need to check here again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows create_user_context to remain non-blocking if odp is disabled. Seems like a worthwhile optimization?

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewleap-optimizely I see. Probably not in server sdks. They'll be blocked anyway in following decide call even if avoid it here :)

Copy link
Contributor

@Mat001 Mat001 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. Tested fetch without calling opti config.

@andrewleap-optimizely andrewleap-optimizely merged commit 7b1c3f1 into master Mar 20, 2023
@andrewleap-optimizely andrewleap-optimizely deleted the aleap/block_odp_calls_on_datafile branch March 20, 2023 17:21
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.

None yet

3 participants