Skip to content
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.

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

fix: use a growable bloom filter implementation. #8

Merged
merged 4 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
213 changes: 127 additions & 86 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ clap-verbosity-flag = "2.2.1"
color-eyre = "0.6.3"
dirs = "5.0.1"
eyre = "0.6.12"
fastbloom = "0.7.1"
futures = "0.3.30"
growable-bloom-filter = "2.1.1"
indexmap = { version = "2.5.0", features = ["serde"] }
indicatif = "0.17.8"
nonempty = "0.10.0"
Expand All @@ -40,14 +40,15 @@ ssh2 = "0.9.4"
tar = "0.4.41"
tempfile = "3.12.0"
tes = { version = "0.3.0", features = ["client", "serde"] }
thiserror = "2.0.11"
tokio = { version = "1.40.0", features = ["full", "time", "tracing"] }
tokio-metrics = "0.3.1"
tokio-stream = "0.1.16"
toml = "0.8.19"
tracing = "0.1.40"
tracing-log = "0.2.0"
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
url = "2.5.2"
url = { version = "2.5.2", features = ["serde"] }
uuid = { version = "1.10.0", features = [
"v4",
"fast-rng",
Expand Down
4 changes: 4 additions & 0 deletions crankshaft-config/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

* Adds the initial version of the crate.

### Changed

* Use `thiserror` for custom error types ([#8](https://github.com/stjude-rust-labs/crankshaft/pull/8)).
1 change: 1 addition & 0 deletions crankshaft-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ config = "0.14.0"
dirs.workspace = true
regex.workspace = true
serde.workspace = true
thiserror.workspace = true
url.workspace = true

[lints]
Expand Down
58 changes: 33 additions & 25 deletions crankshaft-config/src/backend/generic.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Configuration related to _generic_ execution backends.

use std::borrow::Cow;
use std::collections::HashMap;
use std::sync::LazyLock;

Expand All @@ -9,25 +10,19 @@ use regex::Captures;
use regex::Regex;
use serde::Deserialize;
use serde::Serialize;
use thiserror::Error;

pub mod driver;

/// An error related to unexpected remaining substitution tokens in a (otherwise
/// presumed to be fully resolved) command.
#[derive(Debug)]
#[derive(Error, Debug)]
#[error("unresolved substitutions in command `{command}`")]
pub struct UnresolvedSubstitutionError {
/// The command containing the unresolved substitutions.
command: String,
}

impl std::fmt::Display for UnresolvedSubstitutionError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "unresolved substitutions in command: {}", self.command)
}
}

impl std::error::Error for UnresolvedSubstitutionError {}

/// A result from substitutions that might contain a
/// [`UnresolvedSubstitutionError`].
pub type ResolveResult = std::result::Result<String, UnresolvedSubstitutionError>;
Expand All @@ -45,7 +40,7 @@ static PLACEHOLDER_REGEX: LazyLock<Regex> = LazyLock::new(|| {
});

/// Replaces placeholders within a generic configuration value.
pub fn substitute(input: &str, replacements: &HashMap<String, String>) -> String {
pub fn substitute(input: &str, replacements: &HashMap<Cow<'_, str>, Cow<'_, str>>) -> String {
PLACEHOLDER_REGEX
.replace_all(input, |captures: &Captures<'_>| {
// SAFETY: the `PLACEHOLDER_REGEX` above is hardcoded to ensure a group
Expand All @@ -54,8 +49,8 @@ pub fn substitute(input: &str, replacements: &HashMap<String, String>) -> String

replacements
.get(key.as_str())
.unwrap_or(&format!("~{{{}}}", key.as_str()))
.to_string()
.map(|r| r.as_ref().to_string())
.unwrap_or_else(|| format!("~{{{key}}}", key = key.as_str()))
})
.to_string()
}
Expand Down Expand Up @@ -91,8 +86,9 @@ pub struct Config {
kill: String,

/// The runtime attributes.
#[builder(into)]
attributes: Option<HashMap<String, String>>,
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
#[builder(into, default)]
attributes: HashMap<Cow<'static, str>, Cow<'static, str>>,
}

impl Config {
Expand All @@ -109,13 +105,16 @@ impl Config {
/// entire runtime attributes HashMap each time a substitution was
/// performed, we first do substitution of the script followed by
/// substitution of the runtime attributes.
// TODO(clay): could this be used with `Cow<'a, str>`?
#[inline]
fn resolve(&self, command: &str, substitutions: &HashMap<String, String>) -> ResolveResult {
fn resolve(
&self,
command: &str,
substitutions: &HashMap<Cow<'_, str>, Cow<'_, str>>,
) -> ResolveResult {
let mut result = substitute(command, substitutions);

if let Some(attrs) = self.attributes() {
result = substitute(&result, attrs);
if !self.attributes.is_empty() {
result = substitute(&result, &self.attributes);
}

// NOTE: this is just to help clean up some of the output. The intention
Expand Down Expand Up @@ -162,23 +161,32 @@ impl Config {
}

/// Gets the runtime attributes.
pub fn attributes(&self) -> Option<&HashMap<String, String>> {
self.attributes.as_ref()
pub fn attributes(&self) -> &HashMap<Cow<'static, str>, Cow<'static, str>> {
&self.attributes
}

/// Gets the submit command with all of the substitutions resolved.
pub fn resolve_submit(&self, substitutions: &HashMap<String, String>) -> ResolveResult {
pub fn resolve_submit(
&self,
substitutions: &HashMap<Cow<'_, str>, Cow<'_, str>>,
) -> ResolveResult {
self.resolve(&self.submit, substitutions)
}

/// Gets the monitor command with all of the substitutions resolved.
pub fn resolve_monitor(&self, substitutions: &HashMap<String, String>) -> ResolveResult {
pub fn resolve_monitor(
&self,
substitutions: &HashMap<Cow<'_, str>, Cow<'_, str>>,
) -> ResolveResult {
self.resolve(&self.monitor, substitutions)
}

/// Gets the kill command with all of the substitutions resolved.
pub fn resolve_kill(&self, substitutions: HashMap<String, String>) -> ResolveResult {
self.resolve(&self.kill, &substitutions)
pub fn resolve_kill(
&self,
substitutions: &HashMap<Cow<'_, str>, Cow<'_, str>>,
) -> ResolveResult {
self.resolve(&self.kill, substitutions)
}
}

Expand All @@ -194,7 +202,7 @@ mod tests {
#[test]
fn replacement_works() -> Result<(), Box<dyn std::error::Error>> {
let mut replacements = HashMap::new();
replacements.insert(String::from("foo"), String::from("bar"));
replacements.insert("foo".into(), "bar".into());

assert_eq!(substitute("hello, ~{foo}", &replacements), "hello, bar");

Expand Down
160 changes: 0 additions & 160 deletions crankshaft-config/src/backend/generic/builder.rs

This file was deleted.

6 changes: 6 additions & 0 deletions crankshaft-docker/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

* Adds the initial version of the crate.

### Changed

* Use `thiserror` for custom error types ([#8](https://github.com/stjude-rust-labs/crankshaft/pull/8)).
* Separate `program` from `args` in container builder ([#8](https://github.com/stjude-rust-labs/crankshaft/pull/8)).
* Replaced `attached` with separate stdout and stderr attach flags ([#8](https://github.com/stjude-rust-labs/crankshaft/pull/8)).
2 changes: 2 additions & 0 deletions crankshaft-docker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ clap = { workspace = true, optional = true }
clap-verbosity-flag = { workspace = true, optional = true }
eyre = { workspace = true, optional = true }
futures.workspace = true
indexmap = { workspace = true }
serde.workspace = true
shlex = { workspace = true, optional = true }
tar.workspace = true
thiserror = { workspace = true }
tokio = { workspace = true, optional = true }
tokio-stream.workspace = true
tracing.workspace = true
Expand Down
Loading