Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Conversation

leoromanovsky
Copy link
Member

@leoromanovsky leoromanovsky commented Oct 6, 2023


labels: mergeable

Fixes: #issue

Motivation and Context

Add protection for exceptions in our SDKs from bubbling out to our customers' applications by wrapping all public methods in a try/catch.

Description

  • is_graceful_mode is on by default - this is prevent exceptions, expect for ValueError from bubbling up as this is the way python performs validation. we throw this exception if the flagKey is not set in validate_not_blank
  • Add tests for mode on and off

How has this been tested?

@leoromanovsky leoromanovsky marked this pull request as ready for review October 6, 2023 18:12
Copy link
Contributor

@giorgiomartini0 giorgiomartini0 left a comment

Choose a reason for hiding this comment

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

Code changes look good to me. I noticed that all the get_<type>_assignment methods are identical other than the last argument to get_assignment_variation, so maybe this could be refactored to reduce code duplication (especially if error handling becomes more complex in the future). Not needed for this PR.

api_key: str
base_url: str = "https://fscdn.eppo.cloud/api"
assignment_logger: AssignmentLogger
is_graceful_mode: bool = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, is this intended for customers to set? I.e. will this need an update in the docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes they can set this, for example in dev mode

@leoromanovsky leoromanovsky merged commit 4ff7636 into main Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants