Skip to content

feat(payload): adopt Probability for ValueConf::float_probability #1878

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.

Sign up for GitHub

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

Open
goxberry wants to merge 1 commit into
base: goxberry/probability-timestamp-config
Choose a base branch
from
+9 −9
Open

feat(payload): adopt Probability for ValueConf::float_probability #1878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions lading_payload/src/dogstatsd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ impl Default for MetricWeights {
#[serde(deny_unknown_fields)]
#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
pub struct ValueConf {
/// Odds out of 256 that the value will be a float and not an integer.
float_probability: f32,
/// Probability that the value will be a float and not an integer.
float_probability: Probability,
range: ConfRange<i64>,
}

impl ValueConf {
/// Create a new instance of `ValueConf` according to the args
#[must_use]
pub fn new(float_probability: f32, range: ConfRange<i64>) -> Self {
pub fn new(float_probability: Probability, range: ConfRange<i64>) -> Self {
Self {
float_probability,
range,
Expand All @@ -136,7 +136,7 @@ impl ValueConf {
impl Default for ValueConf {
fn default() -> Self {
Self {
float_probability: 0.5, // 50%
float_probability: Probability::try_new(0.5).expect("0.5 is in [0.0, 1.0]"),

@chatgpt-codex-connector chatgpt-codex-connector Bot May 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove the panic path from the default config

The repository instructions for /workspace/lading explicitly say production code must not use .unwrap() or .expect(), but this new default value adds an .expect() in ValueConf::default. Even though the current literal is valid, this leaves a panic path in production initialization and violates the no-panic invariant for config construction; prefer a non-panicking constant/helper for the known-valid default probability.

Useful? React with 👍 / 👎.

@blt blt May 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm hmm hmm same comment as in #1877 (comment) but here it's 50%. This may just be a limit of Rust's type system we have to accept.

range: ConfRange::Inclusive {
min: i64::MIN,
max: i64::MAX,
Expand Down
10 changes: 5 additions & 5 deletions lading_payload/src/dogstatsd/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rand::{
prelude::Distribution,
};

use crate::{Error, Generator};
use crate::{Error, Generator, common::config::Probability};

use super::{ConfRange, ValueConf};

Expand All @@ -21,12 +21,12 @@ pub enum NumValue {
#[derive(Clone, Debug)]
pub(crate) enum NumValueGenerator {
Constant {
float_probability: f32,
float_probability: Probability,
int: i64,
float: f64,
},
Uniform {
float_probability: f32,
float_probability: Probability,
int_distr: Uniform<i64>,
float_distr: Uniform<f64>,
},
Expand Down Expand Up @@ -67,7 +67,7 @@ impl<'a> Generator<'a> for NumValueGenerator {
int,
float,
} => {
if prob < *float_probability {
if prob < float_probability.get() {
Ok(NumValue::Float(*float))
} else {
Ok(NumValue::Int(*int))
Expand All @@ -78,7 +78,7 @@ impl<'a> Generator<'a> for NumValueGenerator {
int_distr,
float_distr,
} => {
if prob < *float_probability {
if prob < float_probability.get() {
Ok(NumValue::Float(float_distr.sample(rng)))
} else {
Ok(NumValue::Int(int_distr.sample(rng)))
Expand Down
Loading