Skip to content

[FSSDK-11212] Update code to retry web API calls for fetching datafile and pushing events #445

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Feb 26, 2025
Merged

[FSSDK-11212] Update code to retry web API calls for fetching datafile and pushing events #445

merged 14 commits into from
Feb 26, 2025

Conversation

pvcraven
Copy link
Contributor

@pvcraven pvcraven commented Feb 24, 2025 •
edited
Loading

Summary

  • Retry connections for posting events and pulling datafiles.
    • By default, 3 attempts to get datafile. (Configurable)
    • Twice to send event data. (Non-configurable)
  • Fix for mypy issue, as mypy updated and is no longer happy

Test plan

  • Unit tests
  • Test actual sdk

Issues

jaeopt
jaeopt requested changes Feb 25, 2025
View reviewed 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 except for the wrong value for event retries

Mat001
Mat001 approved these changes Feb 25, 2025
View reviewed changes
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.

lgtm

pvcraven and others added 2 commits February 25, 2025 14:50
@pvcraven @jaeopt
Update optimizely/event_dispatcher.py
8d7d22f 
Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>
Update event dispatch to try three times to send events
20a1f20
@pvcraven pvcraven requested a review from jaeopt February 25, 2025 20:52
jaeopt
jaeopt approved these changes Feb 25, 2025
View reviewed 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.

LGTM

@pvcraven pvcraven merged commit d098f9a into master Feb 26, 2025
23 checks passed
@pvcraven pvcraven deleted the pvcraven-FSSDK-11212 branch February 26, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mat001 Mat001 Mat001 approved these changes

@jaeopt jaeopt jaeopt approved these changes

Assignees

@pvcraven pvcraven

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pvcraven @Mat001 @jaeopt