This repository has been archived by the owner on Jun 7, 2024. It is now read-only.
[EN-17858] FetchConfigurationsTask is more fault tolerant #5
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
simondale00
commented
Feb 8, 2024
Comment on lines
+23
to
+28
| try { | ||
| configurationStore.fetchAndSetExperimentConfiguration(); | ||
| } catch (Exception e) { | ||
| log.error("[Eppo SDK] Error fetching experiment configuration", e); | ||
| } | ||
|
|
There was a problem hiding this comment.
Wouldn't something like ScheduledExecutorService be more fault tolerant?
Yes! ScheduledExecutorService has a lot of benefits over TimerTask, including fault tolerance.... However, you need to shutdown the executor service when terminating the application. This felt like a heavier change, as we'd need to change the EppoClient to either accept a scheduler, or have it's own lifecycle methods to properly shut down the executor.
Given our core issue was handling uncaught runtime exceptions, this felt like a surgical/safe change.
…can be merged back to the Eppo main repo
nt3rp
approved these changes
Feb 8, 2024
|
@nt3rp I was planning on opening a PR w/ this commit against their repo. I could always ping them in our shared slack channel as well. |
xethorn
approved these changes
Feb 8, 2024
simondale00
added a commit
that referenced
this pull request
Feb 8, 2024
* [EN-17858] FetchConfigurationsTask is more fault tolerant * revert change to pom.xml, thereby having a single, clean commit that can be merged back to the Eppo main repo
simondale00
added a commit
that referenced
this pull request
May 8, 2024
simondale00
added a commit
that referenced
this pull request
May 8, 2024
* update gitignore for modern IntelliJ * change artificat ID for beta * Bandit random action placeholder (#1) * ability to pass in assignment options and attributes * simple test case * placeholder bandit algorithm in place * small changes from self-review * more helpful RAC read failure message * WIP wiring up logging * log bandit info as well * preserve bandit test data * feedback from PR and adjust indention to match rest of project * use Math.abs() as its more readable * trailing newlines * Bandit as a dynamic variant (#2) * use dynamic variations * separate bandit logger * ability to log non-bandit selected control * log variation not bandit boolean * remove now unused class * feedback from PR * Deserialize and store bandit parameters (#3) * deserialize bandit parameters * cleaned up * updated to break out subject and action attributes * genericize configuration requestor * changes from self-reivew of PR * Falcon model for bandit parameter evaluation (#4) * work in progress * math in place * working on tests still * tests passing * new test cases * wip manually verifying math * tests passing * remove logging * clean up score computation * changes from self-reivew of PR * consolidate test bandit rac setup code * clean up test file * update comments * more comment improvements * feedback from PR; mainly cleanup and adding some tuning parameters * Have bandit logger separate out context (#5) * work in progress * math in place * working on tests still * tests passing * new test cases * wip manually verifying math * tests passing * remove logging * clean up score computation * changes from self-reivew of PR * consolidate test bandit rac setup code * clean up test file * update comments * more comment improvements * failing tests in place * break out attributes by type * changes from self-review of PR * return type as provided * make EppoValue constructor private * feedback from PR * more fault tolerant reading of properties file (#6) * catch all exceptions; unit test (#7) * Rename BANDIT algorithm type to CONTEXTUAL_BANDIT to match Eppo (#8) * catch all exceptions; unit test * conform bandit algorithm type * Add test for cold start (#9) * explicit test for cold start * upgrade gcloud to appease linter * update deserializer test * Convenience methods to serialize attributes to JSON string and other cleanup (#10) * add back in isXXX checks * helper function for JSON string serialization * rename variation to banditKey for bandit logging * remove accidentally added junit * Prepare beta SDK for merge into Main one (#11) * add graceful mode by default that does not throw exceptions (FF-949) (Eppo-exp#29) * add graceful mode by default that does not throw exceptions (FF-949) * test graceful mode on and off * test all functions * remove from javadoc * Bump com.github.tomakehurst:wiremock-jre8 from 2.33.2 to 2.35.1 (Eppo-exp#28) Bumps [com.github.tomakehurst:wiremock-jre8](https://github.com/wiremock/wiremock) from 2.33.2 to 2.35.1. - [Release notes](https://github.com/wiremock/wiremock/releases) - [Commits](wiremock/wiremock@2.33.2...2.35.1) --- updated-dependencies: - dependency-name: com.github.tomakehurst:wiremock-jre8 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * add compatibility for java 8 (Eppo-exp#35) * java8 compatibility using apache httpclient * cleanup * added timeouts * add maven-gpg-plugin * test across java 8, 11, 17 * pom * remove List.of * fix test * eppo value * fix plugin * profile --------- Co-authored-by: Gaurav Arora <[email protected]> * Bump org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13 (Eppo-exp#36) Bumps org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13. --- updated-dependencies: - dependency-name: org.apache.httpcomponents:httpclient dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [EN-17858] FetchConfigurationsTask is more fault tolerant (#5) (Eppo-exp#32) * [EN-17858] FetchConfigurationsTask is more fault tolerant * revert change to pom.xml, thereby having a single, clean commit that can be merged back to the Eppo main repo * bump to version 2.2.0 (Eppo-exp#38) * add support for semver rule evaluation (FF-1569) (Eppo-exp#39) * add support for semver rule evaluation (FF-1569) * azul builds * adjust unit tests for semver comparison * numeric first * add back in isXXX checks * helper function for JSON string serialization * rename variation to banditKey for bandit logging * remove accidentally added junit --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Simon Dale <[email protected]> * remove duplicate test dependency --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Simon Dale <[email protected]>
simondale00
added a commit
that referenced
this pull request
May 8, 2024
This reverts commit 4f7b91a.
simondale00
added a commit
that referenced
this pull request
May 9, 2024
* Bump com.github.tomakehurst:wiremock-jre8 from 2.33.2 to 2.35.1 (Eppo-exp#28) Bumps [com.github.tomakehurst:wiremock-jre8](https://github.com/wiremock/wiremock) from 2.33.2 to 2.35.1. - [Release notes](https://github.com/wiremock/wiremock/releases) - [Commits](wiremock/wiremock@2.33.2...2.35.1) --- updated-dependencies: - dependency-name: com.github.tomakehurst:wiremock-jre8 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * add compatibility for java 8 (Eppo-exp#35) * java8 compatibility using apache httpclient * cleanup * added timeouts * add maven-gpg-plugin * test across java 8, 11, 17 * pom * remove List.of * fix test * eppo value * fix plugin * profile --------- Co-authored-by: Gaurav Arora <[email protected]> * Bump org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13 (Eppo-exp#36) Bumps org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13. --- updated-dependencies: - dependency-name: org.apache.httpcomponents:httpclient dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [EN-17858] FetchConfigurationsTask is more fault tolerant (#5) (Eppo-exp#32) * [EN-17858] FetchConfigurationsTask is more fault tolerant * revert change to pom.xml, thereby having a single, clean commit that can be merged back to the Eppo main repo * bump to version 2.2.0 (Eppo-exp#38) * add support for semver rule evaluation (FF-1569) (Eppo-exp#39) * add support for semver rule evaluation (FF-1569) * azul builds * adjust unit tests for semver comparison * numeric first * Merge in changes from Beta SDK (Eppo-exp#43) * update gitignore for modern IntelliJ * change artificat ID for beta * Bandit random action placeholder (#1) * ability to pass in assignment options and attributes * simple test case * placeholder bandit algorithm in place * small changes from self-review * more helpful RAC read failure message * WIP wiring up logging * log bandit info as well * preserve bandit test data * feedback from PR and adjust indention to match rest of project * use Math.abs() as its more readable * trailing newlines * Bandit as a dynamic variant (#2) * use dynamic variations * separate bandit logger * ability to log non-bandit selected control * log variation not bandit boolean * remove now unused class * feedback from PR * Deserialize and store bandit parameters (#3) * deserialize bandit parameters * cleaned up * updated to break out subject and action attributes * genericize configuration requestor * changes from self-reivew of PR * Falcon model for bandit parameter evaluation (#4) * work in progress * math in place * working on tests still * tests passing * new test cases * wip manually verifying math * tests passing * remove logging * clean up score computation * changes from self-reivew of PR * consolidate test bandit rac setup code * clean up test file * update comments * more comment improvements * feedback from PR; mainly cleanup and adding some tuning parameters * Have bandit logger separate out context (#5) * work in progress * math in place * working on tests still * tests passing * new test cases * wip manually verifying math * tests passing * remove logging * clean up score computation * changes from self-reivew of PR * consolidate test bandit rac setup code * clean up test file * update comments * more comment improvements * failing tests in place * break out attributes by type * changes from self-review of PR * return type as provided * make EppoValue constructor private * feedback from PR * more fault tolerant reading of properties file (#6) * catch all exceptions; unit test (#7) * Rename BANDIT algorithm type to CONTEXTUAL_BANDIT to match Eppo (#8) * catch all exceptions; unit test * conform bandit algorithm type * Add test for cold start (#9) * explicit test for cold start * upgrade gcloud to appease linter * update deserializer test * Convenience methods to serialize attributes to JSON string and other cleanup (#10) * add back in isXXX checks * helper function for JSON string serialization * rename variation to banditKey for bandit logging * remove accidentally added junit * Prepare beta SDK for merge into Main one (#11) * add graceful mode by default that does not throw exceptions (FF-949) (Eppo-exp#29) * add graceful mode by default that does not throw exceptions (FF-949) * test graceful mode on and off * test all functions * remove from javadoc * Bump com.github.tomakehurst:wiremock-jre8 from 2.33.2 to 2.35.1 (Eppo-exp#28) Bumps [com.github.tomakehurst:wiremock-jre8](https://github.com/wiremock/wiremock) from 2.33.2 to 2.35.1. - [Release notes](https://github.com/wiremock/wiremock/releases) - [Commits](wiremock/wiremock@2.33.2...2.35.1) --- updated-dependencies: - dependency-name: com.github.tomakehurst:wiremock-jre8 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * add compatibility for java 8 (Eppo-exp#35) * java8 compatibility using apache httpclient * cleanup * added timeouts * add maven-gpg-plugin * test across java 8, 11, 17 * pom * remove List.of * fix test * eppo value * fix plugin * profile --------- Co-authored-by: Gaurav Arora <[email protected]> * Bump org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13 (Eppo-exp#36) Bumps org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13. --- updated-dependencies: - dependency-name: org.apache.httpcomponents:httpclient dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [EN-17858] FetchConfigurationsTask is more fault tolerant (#5) (Eppo-exp#32) * [EN-17858] FetchConfigurationsTask is more fault tolerant * revert change to pom.xml, thereby having a single, clean commit that can be merged back to the Eppo main repo * bump to version 2.2.0 (Eppo-exp#38) * add support for semver rule evaluation (FF-1569) (Eppo-exp#39) * add support for semver rule evaluation (FF-1569) * azul builds * adjust unit tests for semver comparison * numeric first * add back in isXXX checks * helper function for JSON string serialization * rename variation to banditKey for bandit logging * remove accidentally added junit --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Simon Dale <[email protected]> * remove duplicate test dependency --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Simon Dale <[email protected]> * Address Action Warnings (Eppo-exp#44) * address action warnings * wiremock-jre8 v3 doesn't play nice with jre8 :( * Get tests working correctly on ubuntu-latest platform (Eppo-exp#45) * bump down mockito-core version to one that JRE8 supports * use same base image as publish * bump mockito version down even more * mockito-core IS mockito * change mocking strategy * logging * more logging * more logging * change test function visibility * is it RELEASE version * roll test way back * more rolling back * make app details tests public * hail mary settings * hail mary2 * use real junit * try osx to spot check * try specifying surefire plugin version * inch back towards goal * changes from self-review of PR * Latest version of staging plugin and increased timeout (Eppo-exp#46) * latest version of staging plugin and increased timeout * its not a github timeout * Add debug logging to publish (Eppo-exp#47) * Add debug logging to publish * add repositry and staging close * restore gpg pass (Eppo-exp#48) * Do not close the staging repository (Eppo-exp#49) * Update endpoint; separate bandit functions (Eppo-exp#50) * use new endpoint and separate bandit methods * bump version * fix access modifier * Revert "[EN-17473] Propagate the variation name to the AssignmentLogData (#4)" This reverts commit a6607bb. * Revert "Makes new value available in AssignmentLogData" This reverts commit 1ffb62a. * Revert "Adds new method to get assignment data" This reverts commit e9c45fc. * Revert "[EN-17858] FetchConfigurationsTask is more fault tolerant (#5)" This reverts commit 4f7b91a. * [EN-20691] Added a getAssignmentVariation function * removed test * Revert "[EN-20691] Added a getAssignmentVariation function" This reverts commit 3881b5d. * Only test against Java 11, since we're only building against Java 11 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Aaron Silverman <[email protected]> Co-authored-by: Aaron Silverman <[email protected]>
simondale00
added a commit
that referenced
this pull request
May 13, 2024
simondale00
added a commit
that referenced
this pull request
May 13, 2024
* update gitignore for modern IntelliJ * change artificat ID for beta * Bandit random action placeholder (#1) * ability to pass in assignment options and attributes * simple test case * placeholder bandit algorithm in place * small changes from self-review * more helpful RAC read failure message * WIP wiring up logging * log bandit info as well * preserve bandit test data * feedback from PR and adjust indention to match rest of project * use Math.abs() as its more readable * trailing newlines * Bandit as a dynamic variant (#2) * use dynamic variations * separate bandit logger * ability to log non-bandit selected control * log variation not bandit boolean * remove now unused class * feedback from PR * Deserialize and store bandit parameters (#3) * deserialize bandit parameters * cleaned up * updated to break out subject and action attributes * genericize configuration requestor * changes from self-reivew of PR * Falcon model for bandit parameter evaluation (#4) * work in progress * math in place * working on tests still * tests passing * new test cases * wip manually verifying math * tests passing * remove logging * clean up score computation * changes from self-reivew of PR * consolidate test bandit rac setup code * clean up test file * update comments * more comment improvements * feedback from PR; mainly cleanup and adding some tuning parameters * Have bandit logger separate out context (#5) * work in progress * math in place * working on tests still * tests passing * new test cases * wip manually verifying math * tests passing * remove logging * clean up score computation * changes from self-reivew of PR * consolidate test bandit rac setup code * clean up test file * update comments * more comment improvements * failing tests in place * break out attributes by type * changes from self-review of PR * return type as provided * make EppoValue constructor private * feedback from PR * more fault tolerant reading of properties file (#6) * catch all exceptions; unit test (#7) * Rename BANDIT algorithm type to CONTEXTUAL_BANDIT to match Eppo (#8) * catch all exceptions; unit test * conform bandit algorithm type * Add test for cold start (#9) * explicit test for cold start * upgrade gcloud to appease linter * update deserializer test * Convenience methods to serialize attributes to JSON string and other cleanup (#10) * add back in isXXX checks * helper function for JSON string serialization * rename variation to banditKey for bandit logging * remove accidentally added junit * Prepare beta SDK for merge into Main one (#11) * add graceful mode by default that does not throw exceptions (FF-949) (Eppo-exp#29) * add graceful mode by default that does not throw exceptions (FF-949) * test graceful mode on and off * test all functions * remove from javadoc * Bump com.github.tomakehurst:wiremock-jre8 from 2.33.2 to 2.35.1 (Eppo-exp#28) Bumps [com.github.tomakehurst:wiremock-jre8](https://github.com/wiremock/wiremock) from 2.33.2 to 2.35.1. - [Release notes](https://github.com/wiremock/wiremock/releases) - [Commits](wiremock/wiremock@2.33.2...2.35.1) --- updated-dependencies: - dependency-name: com.github.tomakehurst:wiremock-jre8 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * add compatibility for java 8 (Eppo-exp#35) * java8 compatibility using apache httpclient * cleanup * added timeouts * add maven-gpg-plugin * test across java 8, 11, 17 * pom * remove List.of * fix test * eppo value * fix plugin * profile --------- Co-authored-by: Gaurav Arora <[email protected]> * Bump org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13 (Eppo-exp#36) Bumps org.apache.httpcomponents:httpclient from 4.5.10 to 4.5.13. --- updated-dependencies: - dependency-name: org.apache.httpcomponents:httpclient dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [EN-17858] FetchConfigurationsTask is more fault tolerant (#5) (Eppo-exp#32) * [EN-17858] FetchConfigurationsTask is more fault tolerant * revert change to pom.xml, thereby having a single, clean commit that can be merged back to the Eppo main repo * bump to version 2.2.0 (Eppo-exp#38) * add support for semver rule evaluation (FF-1569) (Eppo-exp#39) * add support for semver rule evaluation (FF-1569) * azul builds * adjust unit tests for semver comparison * numeric first * add back in isXXX checks * helper function for JSON string serialization * rename variation to banditKey for bandit logging * remove accidentally added junit --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Simon Dale <[email protected]> * remove duplicate test dependency --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Leo Romanovsky <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gaurav Arora <[email protected]> Co-authored-by: Simon Dale <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
https://glossgenius.atlassian.net/browse/EN-17858
try/catchin theFetchConfigurationsTaskto be more fault tolerantRuntimeExceptionwould cause the timer thread to crash, and the SDK->EppoCloud connection would be severed, unbeknownst to the SDK user.(long)Math.random()was always being cast to0.