Skip to content

Commit

Permalink
refactor(core): make Configuration implementation private (#180)
Browse files Browse the repository at this point in the history
  • Loading branch information
rasendubi authored Jan 24, 2025
1 parent 2a661b3 commit 02a310d
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 48 deletions.
9 changes: 9 additions & 0 deletions .changeset/tender-wombats-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"eppo_core": major
---

[core] Refactor: make Configuration implementation private.

This allows further evolution of configuration without breaking users.

The change should be invisible to SDKs.
45 changes: 37 additions & 8 deletions eppo_core/src/configuration.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashSet;
use std::borrow::Cow;

use chrono::{DateTime, Utc};

Expand All @@ -12,11 +12,11 @@ use crate::{
#[derive(Debug)]
pub struct Configuration {
/// Timestamp when configuration was fetched by the SDK.
pub fetched_at: DateTime<Utc>,
pub(crate) fetched_at: DateTime<Utc>,
/// Flags configuration.
pub flags: UniversalFlagConfig,
pub(crate) flags: UniversalFlagConfig,
/// Bandits configuration.
pub bandits: Option<BanditResponse>,
pub(crate) bandits: Option<BanditResponse>,
}

impl Configuration {
Expand Down Expand Up @@ -51,9 +51,38 @@ impl Configuration {
self.bandits.as_ref()?.bandits.get(bandit_key)
}

/// Get a set of all available flags. Note that this may return both disabled flags and flags
/// with bad configuration.
pub fn flag_keys(&self) -> HashSet<Str> {
self.flags.compiled.flags.keys().cloned().collect()
/// Returns an iterator over all flag keys. Note that this may return both disabled flags and
/// flags with bad configuration. Mostly useful for debugging.
pub fn flag_keys(&self) -> impl Iterator<Item = &Str> {
self.flags.compiled.flags.keys()
}

/// Returns an iterator over all bandit keys. Mostly useful to debugging.
pub fn bandit_keys(&self) -> impl Iterator<Item = &Str> {
self.bandits.iter().flat_map(|it| it.bandits.keys())
}

/// Returns bytes representing flags configuration.
///
/// The return value should be treated as opaque and passed on to another Eppo client for
/// initialization.
pub fn get_flags_configuration(&self) -> Option<Cow<[u8]>> {
Some(Cow::Borrowed(self.flags.to_json()))
}

/// Returns bytes representing bandits configuration.
///
/// The return value should be treated as opaque and passed on to another Eppo client for
/// initialization.
pub fn get_bandits_configuration(&self) -> Option<Cow<[u8]>> {
let bandits = self.bandits.as_ref()?;
let json = serde_json::to_vec(bandits);

// TODO: replace with .inspect_err() once we bump MSRV to 1.76
if let Err(err) = &json {
log::warn!(target: "eppo", "failed to serialize bandits: {err:?}");
}

json.ok().map(Cow::Owned)
}
}
2 changes: 1 addition & 1 deletion python-sdk/python/eppo_client/_eppo_client.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Configuration:
def __init__(
self, *, flags_configuration: bytes, bandits_configuration: bytes | None = None
) -> None: ...
def get_flags_configuration(self) -> bytes: ...
def get_flags_configuration(self) -> bytes | None: ...
def get_bandits_configuration(self) -> bytes | None: ...
def get_flag_keys(self) -> Set[str]: ...
def get_bandit_keys(self) -> Set[str]: ...
Expand Down
6 changes: 2 additions & 4 deletions python-sdk/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ impl EppoClient {
fn get_flag_keys<'py>(&'py self, py: Python<'py>) -> PyResult<Bound<PySet>> {
let config = self.configuration_store.get_configuration();
match config {
Some(config) => PySet::new_bound(py, &config.flag_keys()),
Some(config) => PySet::new_bound(py, config.flag_keys()),
None => PySet::empty_bound(py),
}
}
Expand All @@ -483,9 +483,7 @@ impl EppoClient {
fn get_bandit_keys<'py>(&'py self, py: Python<'py>) -> PyResult<Bound<PySet>> {
let config = self.configuration_store.get_configuration();
match config {
Some(config) => {
PySet::new_bound(py, config.bandits.iter().flat_map(|it| it.bandits.keys()))
}
Some(config) => PySet::new_bound(py, config.bandit_keys()),
None => PySet::empty_bound(py),
}
}
Expand Down
28 changes: 6 additions & 22 deletions python-sdk/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,44 +50,28 @@ impl Configuration {
// Returns a set of all flag keys that have been initialized.
// This can be useful to debug the initialization process.
fn get_flag_keys<'py>(&'py self, py: Python<'py>) -> PyResult<Bound<PySet>> {
PySet::new_bound(py, &self.configuration.flag_keys())
PySet::new_bound(py, self.configuration.flag_keys())
}

// Returns a set of all bandit keys that have been initialized.
// This can be useful to debug the initialization process.
fn get_bandit_keys<'py>(&'py self, py: Python<'py>) -> PyResult<Bound<PySet>> {
PySet::new_bound(
py,
self.configuration
.bandits
.iter()
.flat_map(|it| it.bandits.keys()),
)
PySet::new_bound(py, self.configuration.bandit_keys())
}

/// Return bytes representing flags configuration.
///
/// It should be treated as opaque and passed on to another Eppo client (e.g., javascript client
/// on frontend) for initialization.
fn get_flags_configuration(&self) -> Cow<[u8]> {
Cow::Borrowed(self.configuration.flags.to_json())
fn get_flags_configuration(&self) -> Option<Cow<[u8]>> {
self.configuration.get_flags_configuration()
}

/// Return bytes representing bandits configuration.
///
/// It should be treated as opaque and passed on to another Eppo client for initialization.
fn get_bandits_configuration(&self) -> PyResult<Option<Cow<[u8]>>> {
self.configuration
.bandits
.as_ref()
.map(|it| serde_json::to_vec(it).map(Cow::Owned))
.transpose()
.map_err(|err| {
// This should normally never happen.
PyRuntimeError::new_err(format!(
"failed to serialize bandits configuration: {err:?}"
))
})
fn get_bandits_configuration(&self) -> Option<Cow<[u8]>> {
self.configuration.get_bandits_configuration()
}
}

Expand Down
24 changes: 11 additions & 13 deletions ruby-sdk/ext/eppo_client/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,22 +81,20 @@ impl Configuration {
Ok(Configuration { inner })
}

fn flags_configuration(ruby: &Ruby, rb_self: &Self) -> Result<RString, Error> {
Ok(ruby.str_from_slice(rb_self.inner.flags.to_json()))
fn flags_configuration(ruby: &Ruby, rb_self: &Self) -> Result<Option<RString>, Error> {
let result = rb_self
.inner
.get_flags_configuration()
.map(|s| ruby.str_from_slice(s.as_ref()));
Ok(result)
}

fn bandits_configuration(ruby: &Ruby, rb_self: &Self) -> Result<Option<RString>, Error> {
let Some(bandits) = &rb_self.inner.bandits else {
return Ok(None);
};
let vec = serde_json::to_vec(bandits).map_err(|err| {
// this should never happen
Error::new(
ruby.exception_runtime_error(),
format!("failed to serialize bandits configuration: {err:?}"),
)
})?;
Ok(Some(ruby.str_from_slice(&vec)))
let result = rb_self
.inner
.get_bandits_configuration()
.map(|s| ruby.str_from_slice(s.as_ref()));
Ok(result)
}
}

Expand Down

0 comments on commit 02a310d

Please sign in to comment.