From a1ac5deb94d85ab1f51272c7769b48647ce9b211 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Sat, 9 Nov 2024 00:32:41 -0800 Subject: [PATCH 01/24] chore: generate rust-sdk github action with build.rs --- .github/workflows/ci.yml | 61 ++++++++++++++++++++++++++-------------- rust-sdk/Cargo.toml | 4 +++ rust-sdk/build.rs | 47 +++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 21 deletions(-) create mode 100644 rust-sdk/build.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bf28d77..16340fc3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,28 +1,47 @@ -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_target: + types: + - opened + - synchronize + - reopened + branches: + - main 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 + - name: Setup Rust Toolchain + uses: actions-rust-lang/setup-rust-toolchain@v1 + with: + toolchain: stable, nightly + components: clippy, rustfmt + - name: Cargo Test + run: cargo test --all-features --workspace + - name: Cargo Fmt + run: cargo +nightly fmt --check + - name: Cargo Clippy + run: cargo +nightly clippy --all-features --workspace -- -D warnings diff --git a/rust-sdk/Cargo.toml b/rust-sdk/Cargo.toml index 606f634a..62552924 100644 --- a/rust-sdk/Cargo.toml +++ b/rust-sdk/Cargo.toml @@ -21,3 +21,7 @@ name = "simple" [dev-dependencies] chrono = "0.4.38" env_logger = { version = "0.11.3", features = ["unstable-kv"] } + +[build-dependencies] +gh-workflow = "0.3.0" +serde_yaml = "0.9" diff --git a/rust-sdk/build.rs b/rust-sdk/build.rs new file mode 100644 index 00000000..5465eed9 --- /dev/null +++ b/rust-sdk/build.rs @@ -0,0 +1,47 @@ +use gh_workflow::*; +use toolchain::Toolchain; + +fn main() { + let build = Job::new("Build and Test Rust SDK") + .add_step(Step::checkout()) + .add_step( + Toolchain::default() + .add_stable() + .add_nightly() + .add_clippy() + .add_fmt(), + ) + .add_step( + Cargo::new("test") + .args("--all-features --workspace") + .name("Cargo Test"), + ) + .add_step( + Cargo::new("fmt") + .nightly() + .args("--check") + .name("Cargo Fmt"), + ) + .add_step( + Cargo::new("clippy") + .nightly() + .args("--all-features --workspace -- -D warnings") + .name("Cargo Clippy"), + ); + + let event = Event::default() + .push(Push::default().add_branch("main")) + .pull_request_target( + PullRequestTarget::default() + .open() + .synchronize() + .reopen() + .add_branch("main"), + ); + + Workflow::new("Build and Test") + .on(event) + .add_job("build", build) + .generate() + .unwrap(); +} From 708f1b90310b5d9f0120b856aef4240cbaae09af Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 11 Nov 2024 21:01:06 -0800 Subject: [PATCH 02/24] use pull_request instead of pull_request_target --- .github/workflows/ci.yml | 8 +------- rust-sdk/build.rs | 8 +------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16340fc3..089d56be 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,13 +20,7 @@ on: push: branches: - main - pull_request_target: - types: - - opened - - synchronize - - reopened - branches: - - main + pull_request: {} jobs: build: name: Build and Test Rust SDK diff --git a/rust-sdk/build.rs b/rust-sdk/build.rs index 5465eed9..005b061c 100644 --- a/rust-sdk/build.rs +++ b/rust-sdk/build.rs @@ -31,13 +31,7 @@ fn main() { let event = Event::default() .push(Push::default().add_branch("main")) - .pull_request_target( - PullRequestTarget::default() - .open() - .synchronize() - .reopen() - .add_branch("main"), - ); + .pull_request(PullRequest::default()); Workflow::new("Build and Test") .on(event) From d78c7d4ef4548c04e1ff66195bd02e1845ff42d9 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 11 Nov 2024 21:15:50 -0800 Subject: [PATCH 03/24] git submodules; npm ci; build and docs --- .github/workflows/ci.yml | 11 +++++++++-- rust-sdk/Cargo.toml | 2 ++ rust-sdk/build.rs | 22 ++++++++++++++-------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 089d56be..7e6273f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,14 +28,21 @@ jobs: steps: - 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, nightly + toolchain: stable components: clippy, rustfmt + - name: Cargo Build + run: cargo build --all-features --all-targets --verbose - name: Cargo Test - run: cargo test --all-features --workspace + run: cargo test --all-features --verbose - name: Cargo Fmt run: cargo +nightly fmt --check - name: Cargo Clippy run: cargo +nightly clippy --all-features --workspace -- -D warnings + - name: Cargo Doc + run: cargo doc --verbose diff --git a/rust-sdk/Cargo.toml b/rust-sdk/Cargo.toml index 62552924..c8a4c6f3 100644 --- a/rust-sdk/Cargo.toml +++ b/rust-sdk/Cargo.toml @@ -25,3 +25,5 @@ env_logger = { version = "0.11.3", features = ["unstable-kv"] } [build-dependencies] gh-workflow = "0.3.0" serde_yaml = "0.9" +serde_json = "1.0.132" +indexmap = "2.6.0" \ No newline at end of file diff --git a/rust-sdk/build.rs b/rust-sdk/build.rs index 005b061c..a4eaea8a 100644 --- a/rust-sdk/build.rs +++ b/rust-sdk/build.rs @@ -1,19 +1,24 @@ 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_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( - Toolchain::default() - .add_stable() - .add_nightly() - .add_clippy() - .add_fmt(), + Cargo::new("build") + .args("--all-features --all-targets --verbose") + .name("Cargo Build"), ) .add_step( Cargo::new("test") - .args("--all-features --workspace") + .args("--all-features --verbose") .name("Cargo Test"), ) .add_step( @@ -27,7 +32,8 @@ fn main() { .nightly() .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")) From bb7374cd5b9a677c971f2a2f5ba10faffcf719a1 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 11 Nov 2024 21:20:54 -0800 Subject: [PATCH 04/24] not nightly clippy or fmt --- .github/workflows/ci.yml | 4 ++-- rust-sdk/build.rs | 8 +------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e6273f6..e0645d16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,8 +41,8 @@ jobs: - name: Cargo Test run: cargo test --all-features --verbose - name: Cargo Fmt - run: cargo +nightly fmt --check + run: cargo fmt --check - name: Cargo Clippy - run: cargo +nightly clippy --all-features --workspace -- -D warnings + run: cargo clippy --all-features --workspace -- -D warnings - name: Cargo Doc run: cargo doc --verbose diff --git a/rust-sdk/build.rs b/rust-sdk/build.rs index a4eaea8a..aa870154 100644 --- a/rust-sdk/build.rs +++ b/rust-sdk/build.rs @@ -21,15 +21,9 @@ fn main() { .args("--all-features --verbose") .name("Cargo Test"), ) - .add_step( - Cargo::new("fmt") - .nightly() - .args("--check") - .name("Cargo Fmt"), - ) + .add_step(Cargo::new("fmt").args("--check").name("Cargo Fmt")) .add_step( Cargo::new("clippy") - .nightly() .args("--all-features --workspace -- -D warnings") .name("Cargo Clippy"), ) From 218fff4830b58c26c18b80f6c3adeabe7743ab8e Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 11 Nov 2024 21:26:38 -0800 Subject: [PATCH 05/24] fix fmt --- ruby-sdk/ext/eppo_client/src/configuration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby-sdk/ext/eppo_client/src/configuration.rs b/ruby-sdk/ext/eppo_client/src/configuration.rs index afcc81b4..2fe881c0 100644 --- a/ruby-sdk/ext/eppo_client/src/configuration.rs +++ b/ruby-sdk/ext/eppo_client/src/configuration.rs @@ -87,7 +87,7 @@ impl Configuration { fn bandits_configuration(ruby: &Ruby, rb_self: &Self) -> Result, 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 From f40a765308185d3e251b9ba9ebba22de389955ab Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:43:09 -0800 Subject: [PATCH 06/24] remove static - https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/book/first-edition/const-and-static.html --- eppo_core/src/configuration_fetcher.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/eppo_core/src/configuration_fetcher.rs b/eppo_core/src/configuration_fetcher.rs index 4e446c09..fdb51c84 100644 --- a/eppo_core/src/configuration_fetcher.rs +++ b/eppo_core/src/configuration_fetcher.rs @@ -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 { From 301d5cd17213df4dc32c3143a3ae7919f34620a5 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:49:47 -0800 Subject: [PATCH 07/24] remove redundant return --- eppo_core/src/configuration_fetcher.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/eppo_core/src/configuration_fetcher.rs b/eppo_core/src/configuration_fetcher.rs index fdb51c84..5c5340a0 100644 --- a/eppo_core/src/configuration_fetcher.rs +++ b/eppo_core/src/configuration_fetcher.rs @@ -64,7 +64,7 @@ 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()?; @@ -106,13 +106,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) } })?; From c63c643655ccea2dc9f69e4909c0b268395c6451 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:52:09 -0800 Subject: [PATCH 08/24] unnecessary borrow --- eppo_core/src/eval/eval_assignment.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eppo_core/src/eval/eval_assignment.rs b/eppo_core/src/eval/eval_assignment.rs index 4096190e..e87266dc 100644 --- a/eppo_core/src/eval/eval_assignment.rs +++ b/eppo_core/src/eval/eval_assignment.rs @@ -106,9 +106,9 @@ pub(super) fn get_assignment_with_visitor( config.flags.compiled.eval_flag( visitor, - &flag_key, - &subject_key, - &subject_attributes, + flag_key, + subject_key, + subject_attributes, expected_type, now, ) From 0b825dc99e35a81138e136285de32b2d06325efd Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:55:42 -0800 Subject: [PATCH 09/24] remove return --- eppo_core/src/eval/eval_bandits.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/eppo_core/src/eval/eval_bandits.rs b/eppo_core/src/eval/eval_bandits.rs index 00e2cf34..ab8b5517 100644 --- a/eppo_core/src/eval/eval_bandits.rs +++ b/eppo_core/src/eval/eval_bandits.rs @@ -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, @@ -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, @@ -232,7 +234,7 @@ fn get_bandit_action_with_visitor( bandit_event: Some(bandit_event), }; visitor.on_result(Ok(()), &result); - return result; + result } impl BanditModelData { From 5a4d86a8c670a7999303cca4a015187f9636e3c5 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:58:12 -0800 Subject: [PATCH 10/24] remove return --- eppo_core/src/configuration_fetcher.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/eppo_core/src/configuration_fetcher.rs b/eppo_core/src/configuration_fetcher.rs index 5c5340a0..dfb5d419 100644 --- a/eppo_core/src/configuration_fetcher.rs +++ b/eppo_core/src/configuration_fetcher.rs @@ -71,13 +71,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) } })?; From c7629c53f665669c01a948fb7587004cc95e1f48 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:59:21 -0800 Subject: [PATCH 11/24] apply clippy fixes to py --- python-sdk/src/client.rs | 22 +++++++++------------- python-sdk/src/configuration.rs | 2 +- python-sdk/src/init.rs | 16 +++++++--------- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/python-sdk/src/client.rs b/python-sdk/src/client.rs index 4da0e4f9..2da4109b 100644 --- a/python-sdk/src/client.rs +++ b/python-sdk/src/client.rs @@ -529,17 +529,15 @@ fn context_attributes_from_py<'py>( if let Ok(attrs) = Attributes::extract_bound(obj) { 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) -> PyResult> { - if let Ok(result) = FromPyObject::extract_bound(&obj) { + if let Ok(result) = FromPyObject::extract_bound(obj) { return Ok(result); } - if let Ok(result) = HashMap::::extract_bound(&obj) { + if let Ok(result) = HashMap::::extract_bound(obj) { let result = result .into_iter() .map(|(name, attrs)| (name, ContextAttributes::from(attrs))) @@ -547,9 +545,7 @@ fn actions_from_py(obj: &Bound) -> PyResult, ) -> PyResult { let result = self.evaluator.get_assignment( - &flag_key, - &subject_key.into(), + flag_key, + &subject_key, &subject_attributes.into(), expected_type, ); @@ -656,8 +652,8 @@ impl EppoClient { default: Py, ) -> PyResult { let (result, event) = self.evaluator.get_assignment_details( - &flag_key, - &subject_key.into(), + flag_key, + &subject_key, &subject_attributes.into(), expected_type, ); diff --git a/python-sdk/src/configuration.rs b/python-sdk/src/configuration.rs index 46e80686..a00ea546 100644 --- a/python-sdk/src/configuration.rs +++ b/python-sdk/src/configuration.rs @@ -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:?}")) diff --git a/python-sdk/src/init.rs b/python-sdk/src/init.rs index 9e674fdc..ef64aa44 100644 --- a/python-sdk/src/init.rs +++ b/python-sdk/src/init.rs @@ -21,7 +21,7 @@ pub fn init(config: Bound) -> PyResult> { 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 = { @@ -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 From 119f42edcac4a3b1f627369e52a6be5054568ab6 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 10:59:53 -0800 Subject: [PATCH 12/24] apply clippy fixes to eppo --- rust-sdk/src/client.rs | 2 +- rust-sdk/src/poller.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rust-sdk/src/client.rs b/rust-sdk/src/client.rs index ee70c1a1..cf336c63 100644 --- a/rust-sdk/src/client.rs +++ b/rust-sdk/src/client.rs @@ -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, diff --git a/rust-sdk/src/poller.rs b/rust-sdk/src/poller.rs index 5cb94a71..7d55f8a4 100644 --- a/rust-sdk/src/poller.rs +++ b/rust-sdk/src/poller.rs @@ -46,7 +46,7 @@ impl PollerThread { let fetcher = ConfigurationFetcher::new(ConfigurationFetcherConfig { base_url: config.base_url, api_key: config.api_key, - sdk_metadata: SDK_METADATA.clone(), + sdk_metadata: SDK_METADATA, }); let inner = PollerThreadImpl::start(fetcher, config.store)?; Ok(PollerThread(inner)) From 2e7d9fefa19eec1f79f5d7613dfa754c9cc04080 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:03:40 -0800 Subject: [PATCH 13/24] lifetimes --- eppo_core/src/eval/eval_details_builder.rs | 6 +++--- eppo_core/src/eval/eval_visitor.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/eppo_core/src/eval/eval_details_builder.rs b/eppo_core/src/eval/eval_details_builder.rs index 302598e5..7b46bad7 100644 --- a/eppo_core/src/eval/eval_details_builder.rs +++ b/eppo_core/src/eval/eval_details_builder.rs @@ -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 } @@ -234,7 +234,7 @@ impl<'b> EvalAssignmentVisitor for &'b mut EvalDetailsBuilder { ::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) } @@ -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 diff --git a/eppo_core/src/eval/eval_visitor.rs b/eppo_core/src/eval/eval_visitor.rs index aef10303..f9fa4d2d 100644 --- a/eppo_core/src/eval/eval_visitor.rs +++ b/eppo_core/src/eval/eval_visitor.rs @@ -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); @@ -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 } @@ -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 } From 01ff7689e75618115eb6d12782932fe8922b8d77 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:04:54 -0800 Subject: [PATCH 14/24] indent --- rust-sdk/src/poller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-sdk/src/poller.rs b/rust-sdk/src/poller.rs index 7d55f8a4..e980f1f7 100644 --- a/rust-sdk/src/poller.rs +++ b/rust-sdk/src/poller.rs @@ -66,7 +66,7 @@ impl PollerThread { /// This method can fail with the following errors: /// /// - [`Error::PollerThreadPanicked`]: If the poller thread panicked while waiting for - /// configuration. + /// configuration. /// /// # Example /// From cedeaf72b4632af2ba25fc3df3a19b185dbc5ccf Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:13:34 -0800 Subject: [PATCH 15/24] remove serde_yaml unused --- rust-sdk/Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rust-sdk/Cargo.toml b/rust-sdk/Cargo.toml index c8a4c6f3..29629087 100644 --- a/rust-sdk/Cargo.toml +++ b/rust-sdk/Cargo.toml @@ -24,6 +24,5 @@ env_logger = { version = "0.11.3", features = ["unstable-kv"] } [build-dependencies] gh-workflow = "0.3.0" -serde_yaml = "0.9" -serde_json = "1.0.132" +serde_json = "1.0.116" indexmap = "2.6.0" \ No newline at end of file From a11ecf6f4bcd20fbc999b0b9bd93b3e65b5d8b6c Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:31:19 -0800 Subject: [PATCH 16/24] ws --- rust-sdk/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-sdk/Cargo.toml b/rust-sdk/Cargo.toml index 29629087..a3773b68 100644 --- a/rust-sdk/Cargo.toml +++ b/rust-sdk/Cargo.toml @@ -25,4 +25,4 @@ env_logger = { version = "0.11.3", features = ["unstable-kv"] } [build-dependencies] gh-workflow = "0.3.0" serde_json = "1.0.116" -indexmap = "2.6.0" \ No newline at end of file +indexmap = "2.6.0" From b64845a96d54ac96ac011d0cdd40c7e3c2528f95 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:31:26 -0800 Subject: [PATCH 17/24] py fmt --- python-sdk/src/client.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python-sdk/src/client.rs b/python-sdk/src/client.rs index 2da4109b..ac888657 100644 --- a/python-sdk/src/client.rs +++ b/python-sdk/src/client.rs @@ -529,7 +529,9 @@ fn context_attributes_from_py<'py>( if let Ok(attrs) = Attributes::extract_bound(obj) { return Ok(RefOrOwned::Owned(attrs.into())); } - Err(PyTypeError::new_err("attributes must be either ContextAttributes or Attributes".to_string())) + Err(PyTypeError::new_err( + "attributes must be either ContextAttributes or Attributes".to_string(), + )) } fn actions_from_py(obj: &Bound) -> PyResult> { @@ -545,7 +547,9 @@ fn actions_from_py(obj: &Bound) -> PyResult Date: Tue, 12 Nov 2024 11:33:14 -0800 Subject: [PATCH 18/24] no borrow --- eppo_core/src/eval/evaluator.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/eppo_core/src/eval/evaluator.rs b/eppo_core/src/eval/evaluator.rs index 431d3f86..4a22b98f 100644 --- a/eppo_core/src/eval/evaluator.rs +++ b/eppo_core/src/eval/evaluator.rs @@ -41,9 +41,9 @@ impl Evaluator { let config = self.get_configuration(); get_assignment( config.as_ref().map(AsRef::as_ref), - &flag_key, - &subject_key, - &subject_attributes, + flag_key, + subject_key, + subject_attributes, expected_type, Utc::now(), ) @@ -62,9 +62,9 @@ impl Evaluator { let config = self.get_configuration(); get_assignment_details( config.as_ref().map(AsRef::as_ref), - &flag_key, - &subject_key, - &subject_attributes, + flag_key, + subject_key, + subject_attributes, expected_type, Utc::now(), ) From 289675142adecddcdc298263b18caa14034e4192 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:34:33 -0800 Subject: [PATCH 19/24] format with evaluators --- eppo_core/src/eval/eval_details_builder.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/eppo_core/src/eval/eval_details_builder.rs b/eppo_core/src/eval/eval_details_builder.rs index 7b46bad7..80847286 100644 --- a/eppo_core/src/eval/eval_details_builder.rs +++ b/eppo_core/src/eval/eval_details_builder.rs @@ -139,10 +139,10 @@ impl EvalDetailsBuilder { } EvaluationFailure::Error(EvaluationError::UnexpectedConfigurationError) | EvaluationFailure::Error(EvaluationError::UnexpectedConfigurationParseError) => { - format!("Configuration error. This might indicate that you're using an outdated version of Eppo SDK") + "Configuration error. This might indicate that you're using an outdated version of Eppo SDK".to_string() } EvaluationFailure::ConfigurationMissing => { - format!("Configuration has not been fetched yet") + "Configuration has not been fetched yet".to_string() } EvaluationFailure::FlagUnrecognizedOrDisabled => { format!("Unrecognized or disabled flag: {}", self.flag_key) @@ -150,22 +150,23 @@ impl EvalDetailsBuilder { EvaluationFailure::FlagDisabled => { format!("Unrecognized or disabled flag: {}", self.flag_key) } - EvaluationFailure::DefaultAllocationNull => format!( + EvaluationFailure::DefaultAllocationNull => { "No allocations matched. Falling back to \"Default Allocation\", serving NULL" - ), + .to_string() + } EvaluationFailure::NonBanditVariation => { debug_assert!( false, "{failure:?} should never be emitted by flag evaluation" ); - format!("Flag evaluated to a non-bandit allocation") + "Flag evaluated to a non-bandit allocation".to_string() } EvaluationFailure::NoActionsSuppliedForBandit => { debug_assert!( false, "{failure:?} should never be emitted by flag evaluation" ); - format!("No actions were supplied for bandit evaluation") + "No actions were supplied for bandit evaluation".to_string() } }; } From 71d6c4e7a9ccd07c6151e75d3bd9c712ac96e1c4 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:36:24 -0800 Subject: [PATCH 20/24] too many args --- eppo_core/src/eval/eval_bandits.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/eppo_core/src/eval/eval_bandits.rs b/eppo_core/src/eval/eval_bandits.rs index ab8b5517..cda52e45 100644 --- a/eppo_core/src/eval/eval_bandits.rs +++ b/eppo_core/src/eval/eval_bandits.rs @@ -108,6 +108,7 @@ pub fn get_bandit_action_details( /// 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)] fn get_bandit_action_with_visitor( visitor: &mut V, configuration: Option<&Configuration>, @@ -380,7 +381,7 @@ fn score_attributes( categorical_coefficients: &[BanditCategoricalAttributeCoefficient], ) -> f64 { numeric_coefficients - .into_iter() + .iter() .map(|coef| { attributes .numeric @@ -390,7 +391,7 @@ fn score_attributes( .map(|value| value * coef.coefficient) .unwrap_or(coef.missing_value_coefficient) }) - .chain(categorical_coefficients.into_iter().map(|coef| { + .chain(categorical_coefficients.iter().map(|coef| { attributes .categorical .get(&coef.attribute_key) From 177caa67f2184ead073e309bb327ca4930081048 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:36:47 -0800 Subject: [PATCH 21/24] is_empty --- eppo_core/src/eval/eval_bandits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_core/src/eval/eval_bandits.rs b/eppo_core/src/eval/eval_bandits.rs index cda52e45..a0baa3fc 100644 --- a/eppo_core/src/eval/eval_bandits.rs +++ b/eppo_core/src/eval/eval_bandits.rs @@ -249,7 +249,7 @@ impl BanditModelData { // total_shards is not configurable at the moment. const TOTAL_SHARDS: u32 = 10_000; - if actions.len() == 0 { + if actions.is_empty() { return Err(EvaluationFailure::NoActionsSuppliedForBandit); } From d1f4f8d2000fb0db6781aeb642c8bb10a77d86b6 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:37:11 -0800 Subject: [PATCH 22/24] no borrow --- eppo_core/src/eval/eval_bandits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_core/src/eval/eval_bandits.rs b/eppo_core/src/eval/eval_bandits.rs index a0baa3fc..5ac221f9 100644 --- a/eppo_core/src/eval/eval_bandits.rs +++ b/eppo_core/src/eval/eval_bandits.rs @@ -363,7 +363,7 @@ impl BanditModelData { coefficients.intercept + score_attributes( - &action.attributes, + action.attributes, &coefficients.action_numeric_coefficients, &coefficients.action_categorical_coefficients, ) From c85d5015b59dc12f3d9630f46e54dccc2bafc855 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:37:43 -0800 Subject: [PATCH 23/24] simplify map_err --- eppo_core/src/configuration_fetcher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_core/src/configuration_fetcher.rs b/eppo_core/src/configuration_fetcher.rs index dfb5d419..633fddc8 100644 --- a/eppo_core/src/configuration_fetcher.rs +++ b/eppo_core/src/configuration_fetcher.rs @@ -98,7 +98,7 @@ impl ConfigurationFetcher { ("coreVersion", env!("CARGO_PKG_VERSION")), ], ) - .map_err(|err| Error::InvalidBaseUrl(err))?; + .map_err(Error::InvalidBaseUrl)?; log::debug!(target: "eppo", "fetching UFC bandits configuration"); let response = self.client.get(url).send()?; From c5763cf449dcc32b43e08aeb323f013a5b55f3cb Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Tue, 12 Nov 2024 11:40:29 -0800 Subject: [PATCH 24/24] no borrow --- eppo_core/src/str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_core/src/str.rs b/eppo_core/src/str.rs index b64d49bc..c5e62c97 100644 --- a/eppo_core/src/str.rs +++ b/eppo_core/src/str.rs @@ -106,7 +106,7 @@ mod pyo3_impl { impl ToPyObject for Str { fn to_object(&self, py: Python<'_>) -> PyObject { - PyString::new_bound(py, &self).into() + PyString::new_bound(py, self).into() } } }