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

feat: Bandits part 5: load and store bandit models from API #28

Merged
merged 120 commits into from
Jul 24, 2024

Conversation

typotter
Copy link
Collaborator

@typotter typotter commented Jul 8, 2024

Motivation and Context

The bandit model data is retrieved from an endpoint different than that used to load the rest of the UFC flag configuration data. Again, similar to previous PRs, we see refactoring in order to leverage existing codepaths for different data types

Description

  • Update the APIRequestWrapper class to load from a second endpoint
  • plumb bandit model data through ConfigurationLoader into the cache and out again

How is this tested

  • unit tests

@@ -36,7 +45,7 @@ public function getFlag(string $key): ?Flag
return null;
}

$inflated = unserialize($result);
$inflated = $result;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

underlying cache layer is already doing the serializing... 🤦

@typotter
Copy link
Collaborator Author

Thanks for the review!
Updated now to only fetchAndStoreBandits when the UFC response has bandits.
Please take another look

@typotter typotter requested a review from aarsilv July 16, 2024 04:47
Copy link
Contributor

@aarsilv aarsilv left a comment

Choose a reason for hiding this comment

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

Thanks for iterating!

*/
private function fetchAndStoreBandits(): void
{
// TODO: implement optimized fetching by checking for expected bandit models from UFC response.
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Comment on lines +120 to +121
// Only load bandits if there are any referenced by the flags.
if ($indexer->hasBandits()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Base automatically changed from tp/bandits/eval to main July 24, 2024 16:20
@typotter typotter merged commit f442dfc into main Jul 24, 2024
1 check passed
@typotter typotter deleted the tp/bandits/models branch July 24, 2024 16:25
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.

2 participants