diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bf28d77..e0645d16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/eppo_core/src/configuration_fetcher.rs b/eppo_core/src/configuration_fetcher.rs index 4e446c09..633fddc8 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 { @@ -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) } })?; @@ -100,20 +98,19 @@ 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()?; 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) } })?; 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, ) diff --git a/eppo_core/src/eval/eval_bandits.rs b/eppo_core/src/eval/eval_bandits.rs index 00e2cf34..5ac221f9 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, @@ -106,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>, @@ -232,7 +235,7 @@ fn get_bandit_action_with_visitor( bandit_event: Some(bandit_event), }; visitor.on_result(Ok(()), &result); - return result; + result } impl BanditModelData { @@ -246,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); } @@ -360,7 +363,7 @@ impl BanditModelData { coefficients.intercept + score_attributes( - &action.attributes, + action.attributes, &coefficients.action_numeric_coefficients, &coefficients.action_categorical_coefficients, ) @@ -378,7 +381,7 @@ fn score_attributes( categorical_coefficients: &[BanditCategoricalAttributeCoefficient], ) -> f64 { numeric_coefficients - .into_iter() + .iter() .map(|coef| { attributes .numeric @@ -388,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) diff --git a/eppo_core/src/eval/eval_details_builder.rs b/eppo_core/src/eval/eval_details_builder.rs index 302598e5..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() } }; } @@ -219,7 +220,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 +235,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 +257,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 } 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(), ) 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() } } } diff --git a/python-sdk/src/client.rs b/python-sdk/src/client.rs index 4da0e4f9..ac888657 100644 --- a/python-sdk/src/client.rs +++ b/python-sdk/src/client.rs @@ -529,17 +529,17 @@ 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 +547,9 @@ 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 +656,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 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 diff --git a/rust-sdk/Cargo.toml b/rust-sdk/Cargo.toml index 606f634a..a3773b68 100644 --- a/rust-sdk/Cargo.toml +++ b/rust-sdk/Cargo.toml @@ -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" +serde_json = "1.0.116" +indexmap = "2.6.0" diff --git a/rust-sdk/build.rs b/rust-sdk/build.rs new file mode 100644 index 00000000..aa870154 --- /dev/null +++ b/rust-sdk/build.rs @@ -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(); +} 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..e980f1f7 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)) @@ -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 ///