Skip to content
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 41 additions & 21 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,28 +1,48 @@
name: Test
# -------------------------------------------------------------------
# ------------------------------- WARNING ---------------------------
# -------------------------------------------------------------------
#
# This file was automatically generated by gh-workflows using the
# gh-workflow-gen bin. You should add and commit this file to your
# git repository. **DO NOT EDIT THIS FILE BY HAND!** Any manual changes
# will be lost if the file is regenerated.
#
# To make modifications, update your `build.rs` configuration to adjust
# the workflow description as needed, then regenerate this file to apply
# those changes.
#
# -------------------------------------------------------------------
# ----------------------------- END WARNING -------------------------
# -------------------------------------------------------------------

name: Build and Test
on:
push:
branches:
- main
pull_request:

env:
CARGO_TERM_COLOR: always

- main
pull_request: {}
jobs:
cargo_build_and_test:
name: Cargo Build & Test
build:
name: Build and Test Rust SDK
runs-on: ubuntu-latest
strategy:
matrix:
toolchain:
- stable
steps:
- uses: actions/checkout@v4
with:
submodules: true
- run: npm ci
- run: rustup update ${{ matrix.toolchain }} && rustup default ${{ matrix.toolchain }}
- run: cargo build --verbose --all-targets
- run: cargo test --verbose
- run: cargo doc --verbose
- name: Checkout Code
uses: actions/checkout@v4
with:
submodules: 'true'
- run: npm ci
- name: Setup Rust Toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: stable
components: clippy, rustfmt
- name: Cargo Build
run: cargo build --all-features --all-targets --verbose
- name: Cargo Test
run: cargo test --all-features --verbose
- name: Cargo Fmt
run: cargo fmt --check
- name: Cargo Clippy
run: cargo clippy --all-features --workspace -- -D warnings
- name: Cargo Doc
run: cargo doc --verbose
35 changes: 16 additions & 19 deletions eppo_core/src/configuration_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ pub struct ConfigurationFetcherConfig {
pub sdk_metadata: SdkMetadata,
}

pub const DEFAULT_BASE_URL: &'static str = "https://fscdn.eppo.cloud/api";

const UFC_ENDPOINT: &'static str = "/flag-config/v1/config";
const BANDIT_ENDPOINT: &'static str = "/flag-config/v1/bandits";
pub const DEFAULT_BASE_URL: &str = "https://fscdn.eppo.cloud/api";
const UFC_ENDPOINT: &str = "/flag-config/v1/config";
const BANDIT_ENDPOINT: &str = "/flag-config/v1/bandits";

/// A client that fetches Eppo configuration from the server.
pub struct ConfigurationFetcher {
Expand Down Expand Up @@ -65,20 +64,19 @@ impl ConfigurationFetcher {
("coreVersion", env!("CARGO_PKG_VERSION")),
],
)
.map_err(|err| Error::InvalidBaseUrl(err))?;
.map_err(Error::InvalidBaseUrl)?;

log::debug!(target: "eppo", "fetching UFC flags configuration");
let response = self.client.get(url).send()?;

let response = response.error_for_status().map_err(|err| {
if err.status() == Some(StatusCode::UNAUTHORIZED) {
log::warn!(target: "eppo", "client is not authorized. Check your API key");
self.unauthorized = true;
return Error::Unauthorized;
} else {
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {:?}", err);
return Error::from(err);

log::warn!(target: "eppo", "client is not authorized. Check your API key");
self.unauthorized = true;
Error::Unauthorized
} else {
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {:?}", err);
Error::from(err)
}
})?;

Expand Down Expand Up @@ -107,13 +105,12 @@ impl ConfigurationFetcher {

let response = response.error_for_status().map_err(|err| {
if err.status() == Some(StatusCode::UNAUTHORIZED) {
log::warn!(target: "eppo", "client is not authorized. Check your API key");
self.unauthorized = true;
return Error::Unauthorized;
} else {
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {:?}", err);
return Error::from(err);

log::warn!(target: "eppo", "client is not authorized. Check your API key");
self.unauthorized = true;
Error::Unauthorized
} else {
log::warn!(target: "eppo", "received non-200 response while fetching new configuration: {:?}", err);
Error::from(err)
}
})?;

Expand Down
6 changes: 3 additions & 3 deletions eppo_core/src/eval/eval_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ pub(super) fn get_assignment_with_visitor<V: EvalAssignmentVisitor>(

config.flags.compiled.eval_flag(
visitor,
&flag_key,
&subject_key,
&subject_attributes,
flag_key,
subject_key,
subject_attributes,
expected_type,
now,
)
Expand Down
4 changes: 3 additions & 1 deletion eppo_core/src/eval/eval_bandits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub struct BanditResult {

/// Evaluate the specified string feature flag for the given subject. If resulting variation is
/// a bandit, evaluate the bandit to return the action.
#[allow(clippy::too_many_arguments)]
pub fn get_bandit_action(
configuration: Option<&Configuration>,
flag_key: &str,
Expand All @@ -73,6 +74,7 @@ pub fn get_bandit_action(

/// Evaluate the specified string feature flag for the given subject. If resulting variation is
/// a bandit, evaluate the bandit to return the action. In addition, return evaluation details.
#[allow(clippy::too_many_arguments)]
pub fn get_bandit_action_details(
configuration: Option<&Configuration>,
flag_key: &str,
Expand Down Expand Up @@ -232,7 +234,7 @@ fn get_bandit_action_with_visitor<V: EvalBanditVisitor>(
bandit_event: Some(bandit_event),
};
visitor.on_result(Ok(()), &result);
return result;
result
}

impl BanditModelData {
Expand Down
6 changes: 3 additions & 3 deletions eppo_core/src/eval/eval_details_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl EvalBanditVisitor for EvalDetailsBuilder {
self.bandit_key = Some(key.to_owned());
}

fn visit_assignment<'a>(&'a mut self) -> Self::AssignmentVisitor<'a> {
fn visit_assignment(&mut self) -> Self::AssignmentVisitor<'_> {
self
}

Expand All @@ -234,7 +234,7 @@ impl<'b> EvalAssignmentVisitor for &'b mut EvalDetailsBuilder {
<EvalDetailsBuilder as EvalAssignmentVisitor>::AllocationVisitor<'a>
where Self: 'a;

fn visit_allocation<'a>(&'a mut self, allocation: &Allocation) -> Self::AllocationVisitor<'a> {
fn visit_allocation(&mut self, allocation: &Allocation) -> Self::AllocationVisitor<'_> {
EvalAssignmentVisitor::visit_allocation(*self, allocation)
}

Expand All @@ -256,7 +256,7 @@ impl EvalAssignmentVisitor for EvalDetailsBuilder {
where
Self: 'a;

fn visit_allocation<'a>(&'a mut self, allocation: &Allocation) -> Self::AllocationVisitor<'a> {
fn visit_allocation(&mut self, allocation: &Allocation) -> Self::AllocationVisitor<'_> {
let order_position = self.allocation_eval_results.len() + 1;
let result = self
.allocation_eval_results
Expand Down
6 changes: 3 additions & 3 deletions eppo_core/src/eval/eval_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(super) trait EvalBanditVisitor {
/// Called when (if) evaluation gets configuration.
fn on_configuration(&mut self, configuration: &Configuration);

fn visit_assignment<'a>(&'a mut self) -> Self::AssignmentVisitor<'a>;
fn visit_assignment(&mut self) -> Self::AssignmentVisitor<'_>;

/// Called when bandit key is known.
fn on_bandit_key(&mut self, key: &str);
Expand Down Expand Up @@ -98,7 +98,7 @@ impl EvalBanditVisitor for NoopEvalVisitor {
fn on_bandit_key(&mut self, _key: &str) {}

#[inline]
fn visit_assignment<'a>(&'a mut self) -> NoopEvalVisitor {
fn visit_assignment(&mut self) -> NoopEvalVisitor {
NoopEvalVisitor
}

Expand All @@ -110,7 +110,7 @@ impl EvalAssignmentVisitor for NoopEvalVisitor {
type AllocationVisitor<'a> = NoopEvalVisitor;

#[inline]
fn visit_allocation<'a>(&'a mut self, _allocation: &Allocation) -> Self::AllocationVisitor<'a> {
fn visit_allocation(&mut self, _allocation: &Allocation) -> Self::AllocationVisitor<'_> {
NoopEvalVisitor
}

Expand Down
22 changes: 9 additions & 13 deletions python-sdk/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,30 +526,26 @@
if let Ok(attrs) = obj.downcast::<ContextAttributes>() {
return Ok(RefOrOwned::Ref(attrs.borrow()));
}
if let Ok(attrs) = Attributes::extract_bound(obj) {

Check warning on line 529 in python-sdk/src/client.rs

View workflow job for this annotation

GitHub Actions / Build and Test Rust SDK

Diff in /home/runner/work/eppo-multiplatform/eppo-multiplatform/python-sdk/src/client.rs
return Ok(RefOrOwned::Owned(attrs.into()));
}
Err(PyTypeError::new_err(format!(
"attributes must be either ContextAttributes or Attributes"
)))
Err(PyTypeError::new_err("attributes must be either ContextAttributes or Attributes".to_string()))
}

fn actions_from_py(obj: &Bound<PyAny>) -> PyResult<HashMap<String, ContextAttributes>> {
if let Ok(result) = FromPyObject::extract_bound(&obj) {
if let Ok(result) = FromPyObject::extract_bound(obj) {
return Ok(result);
}

if let Ok(result) = HashMap::<String, Attributes>::extract_bound(&obj) {
if let Ok(result) = HashMap::<String, Attributes>::extract_bound(obj) {
let result = result
.into_iter()
.map(|(name, attrs)| (name, ContextAttributes::from(attrs)))
.collect();
return Ok(result);

Check warning on line 545 in python-sdk/src/client.rs

View workflow job for this annotation

GitHub Actions / Build and Test Rust SDK

Diff in /home/runner/work/eppo-multiplatform/eppo-multiplatform/python-sdk/src/client.rs
}

Err(PyTypeError::new_err(format!(
"action attributes must be either ContextAttributes or Attributes"
)))
Err(PyTypeError::new_err("action attributes must be either ContextAttributes or Attributes".to_string()))
}

// Rust-only methods
Expand Down Expand Up @@ -599,7 +595,7 @@
.as_ref()
.ok_or_else(|| {
// This should never happen as assigment_logger setter requires a valid logger.
PyRuntimeError::new_err(format!("Config.assignment_logger is None"))
PyRuntimeError::new_err("Config.assignment_logger is None".to_string())
})?
.clone_ref(py),
is_graceful_mode: AtomicBool::new(config.is_graceful_mode),
Expand All @@ -616,8 +612,8 @@
default: Py<PyAny>,
) -> PyResult<PyObject> {
let result = self.evaluator.get_assignment(
&flag_key,
&subject_key.into(),
flag_key,
&subject_key,
&subject_attributes.into(),
expected_type,
);
Expand Down Expand Up @@ -656,8 +652,8 @@
default: Py<PyAny>,
) -> PyResult<EvaluationResult> {
let (result, event) = self.evaluator.get_assignment_details(
&flag_key,
&subject_key.into(),
flag_key,
&subject_key,
&subject_attributes.into(),
expected_type,
);
Expand Down
2 changes: 1 addition & 1 deletion python-sdk/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl Configuration {
PyValueError::new_err(format!("argument 'flags_configuration': {err:?}"))
})?;
let bandits_config = bandits_configuration
.map(|it| serde_json::from_slice(it))
.map(serde_json::from_slice)
.transpose()
.map_err(|err| {
PyValueError::new_err(format!("argument 'bandits_configuration': {err:?}"))
Expand Down
16 changes: 7 additions & 9 deletions python-sdk/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn init(config: Bound<ClientConfig>) -> PyResult<Py<EppoClient>> {

let py = config.py();

let client = Bound::new(py, EppoClient::new(py, &*config.borrow())?)?.unbind();
let client = Bound::new(py, EppoClient::new(py, &config.borrow())?)?.unbind();

// minimizing the scope of holding the write lock
let existing = {
Expand Down Expand Up @@ -77,15 +77,13 @@ fn initialize_pyo3_log() {
// There's a previous handle. Logging is already initialized, but we reset
// caches.
previous_handle.reset();
} else if let Ok(new_handle) = pyo3_log::try_init() {
*reset_handle = Some(new_handle);
} else {
if let Ok(new_handle) = pyo3_log::try_init() {
*reset_handle = Some(new_handle);
} else {
// This should not happen as initialization error signals that we already
// initialized logging. (In which case, `LOG_RESET_HANDLE` should contain
// `Some()`.)
debug_assert!(false, "tried to initialize pyo3_log second time");
}
// This should not happen as initialization error signals that we already
// initialized logging. (In which case, `LOG_RESET_HANDLE` should contain
// `Some()`.)
debug_assert!(false, "tried to initialize pyo3_log second time");
}
} else {
// This should normally never happen as it shows that another thread has panicked
Expand Down
2 changes: 1 addition & 1 deletion ruby-sdk/ext/eppo_client/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl Configuration {

fn bandits_configuration(ruby: &Ruby, rb_self: &Self) -> Result<Option<RString>, Error> {
let Some(bandits) = &rb_self.inner.bandits else {
return Ok(None)
return Ok(None);
};
let vec = serde_json::to_vec(bandits).map_err(|err| {
// this should never happen
Expand Down
5 changes: 5 additions & 0 deletions rust-sdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,8 @@ name = "simple"
[dev-dependencies]
chrono = "0.4.38"
env_logger = { version = "0.11.3", features = ["unstable-kv"] }

[build-dependencies]
gh-workflow = "0.3.0"
Copy link
Member Author

@leoromanovsky leoromanovsky Nov 12, 2024

Choose a reason for hiding this comment

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

❌ A dependency in this package has a MSRV of 1.75 which blocks us from using it: JelteF/derive_more#389

I will fiddle around with it to see if we can overcome the blocker.

serde_json = "1.0.116"
indexmap = "2.6.0"
41 changes: 41 additions & 0 deletions rust-sdk/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use gh_workflow::*;
use indexmap::IndexMap;
use serde_json::json;
use toolchain::Toolchain;

fn main() {
let mut submodules_map = IndexMap::new();
submodules_map.insert("submodules".to_string(), json!("true"));

let build = Job::new("Build and Test Rust SDK")
.add_step(Step::checkout().add_with(submodules_map))
.add_step(Step::run("npm ci"))
.add_step(Toolchain::default().add_stable().add_clippy().add_fmt())
.add_step(
Cargo::new("build")
.args("--all-features --all-targets --verbose")
.name("Cargo Build"),
)
.add_step(
Cargo::new("test")
.args("--all-features --verbose")
.name("Cargo Test"),
)
.add_step(Cargo::new("fmt").args("--check").name("Cargo Fmt"))
.add_step(
Cargo::new("clippy")
.args("--all-features --workspace -- -D warnings")
.name("Cargo Clippy"),
)
.add_step(Cargo::new("doc").args("--verbose").name("Cargo Doc"));

let event = Event::default()
.push(Push::default().add_branch("main"))
.pull_request(PullRequest::default());

Workflow::new("Build and Test")
.on(event)
.add_job("build", build)
.generate()
.unwrap();
}
2 changes: 1 addition & 1 deletion rust-sdk/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl<'a> Client<'a> {
) -> Self {
let evaluator = Evaluator::new(EvaluatorConfig {
configuration_store: configuration_store.clone(),
sdk_metadata: SDK_METADATA.clone(),
sdk_metadata: SDK_METADATA,
});
Self {
configuration_store,
Expand Down
Loading
Loading