Skip to content

Commit

Permalink
cleanup of clap and configure_me from core components
Browse files Browse the repository at this point in the history
  • Loading branch information
milenkovicm committed Dec 11, 2024
1 parent f2f2adc commit 17cb505
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 34 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ tonic-build = { version = "0.12", default-features = false, features = [
"transport",
"prost"
] }
tracing = "0.1.36"
tracing = "0.1"
tracing-appender = "0.2.2"
tracing-subscriber = { version = "0.3.15", features = ["env-filter"] }
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
ctor = { version = "0.2" }
mimalloc = { version = "0.1" }

Expand Down
1 change: 0 additions & 1 deletion ballista-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ keywords = ["ballista", "cli"]
license = "Apache-2.0"
homepage = "https://github.com/apache/arrow-ballista"
repository = "https://github.com/apache/arrow-ballista"
rust-version = "1.72"
readme = "README.md"

[dependencies]
Expand Down
8 changes: 3 additions & 5 deletions ballista/client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,26 @@ repository = "https://github.com/apache/arrow-ballista"
readme = "README.md"
authors = ["Apache DataFusion <[email protected]>"]
edition = "2021"
rust-version = "1.72"

[dependencies]
async-trait = { workspace = true }
ballista-core = { path = "../core", version = "0.12.0" }
ballista-executor = { path = "../executor", version = "0.12.0", optional = true }
ballista-scheduler = { path = "../scheduler", version = "0.12.0", optional = true }
datafusion = { workspace = true }
datafusion-proto = { workspace = true }
futures = { workspace = true }
log = { workspace = true }
parking_lot = { workspace = true }
tempfile = { workspace = true }

tokio = { workspace = true }
url = { workspace = true }

[dev-dependencies]
ballista-executor = { path = "../executor", version = "0.12.0" }
ballista-scheduler = { path = "../scheduler", version = "0.12.0" }
ctor = { workspace = true }
datafusion-proto = { workspace = true }
env_logger = { workspace = true }
rstest = { version = "0.23" }
tempfile = { workspace = true }
tonic = { workspace = true }

[features]
Expand Down
8 changes: 4 additions & 4 deletions ballista/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@ exclude = ["*.proto"]
rustc-args = ["--cfg", "docsrs"]

[features]
build-binary = ["configure_me", "clap"]
docsrs = []
# Used for testing ONLY: causes all values to hash to the same value (test for collisions)
force_hash_collisions = ["datafusion/force_hash_collisions"]


[dependencies]
arrow-flight = { workspace = true }
async-trait = { workspace = true }
chrono = { version = "0.4", default-features = false }
clap = { workspace = true }
configure_me = { workspace = true }
clap = { workspace = true, optional = true }
configure_me = { workspace = true, optional = true }
datafusion = { workspace = true }
datafusion-proto = { workspace = true }
datafusion-proto-common = { workspace = true }
Expand All @@ -65,5 +65,5 @@ url = { workspace = true }
tempfile = { workspace = true }

[build-dependencies]
rustc_version = "0.4.0"
rustc_version = "0.4.1"
tonic-build = { workspace = true }
21 changes: 12 additions & 9 deletions ballista/core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

//! Ballista configuration
use clap::ValueEnum;
use core::fmt;
use std::collections::HashMap;
use std::result;

Expand Down Expand Up @@ -252,30 +250,33 @@ impl datafusion::config::ConfigExtension for BallistaConfig {

// an enum used to configure the scheduler policy
// needs to be visible to code generated by configure_me
#[derive(Clone, ValueEnum, Copy, Debug, serde::Deserialize, Default)]
#[derive(Clone, Copy, Debug, serde::Deserialize, Default)]
#[cfg_attr(feature = "build-binary", derive(clap::ValueEnum))]
pub enum TaskSchedulingPolicy {
#[default]
PullStaged,
PushStaged,
}

#[cfg(feature = "build-binary")]
impl std::str::FromStr for TaskSchedulingPolicy {
type Err = String;

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
ValueEnum::from_str(s, true)
clap::ValueEnum::from_str(s, true)
}
}

#[cfg(feature = "build-binary")]
impl configure_me::parse_arg::ParseArgFromStr for TaskSchedulingPolicy {
fn describe_type<W: fmt::Write>(mut writer: W) -> fmt::Result {
fn describe_type<W: core::fmt::Write>(mut writer: W) -> core::fmt::Result {
write!(writer, "The scheduler policy for the scheduler")
}
}

// an enum used to configure the log rolling policy
// needs to be visible to code generated by configure_me
#[derive(Clone, ValueEnum, Copy, Debug, serde::Deserialize, Default)]
#[derive(Clone, Copy, Debug, serde::Deserialize, Default)]
#[cfg_attr(feature = "build-binary", derive(clap::ValueEnum))]
pub enum LogRotationPolicy {
Minutely,
Hourly,
Expand All @@ -284,16 +285,18 @@ pub enum LogRotationPolicy {
Never,
}

#[cfg(feature = "build-binary")]
impl std::str::FromStr for LogRotationPolicy {
type Err = String;

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
ValueEnum::from_str(s, true)
clap::ValueEnum::from_str(s, true)
}
}

#[cfg(feature = "build-binary")]
impl configure_me::parse_arg::ParseArgFromStr for LogRotationPolicy {
fn describe_type<W: fmt::Write>(mut writer: W) -> fmt::Result {
fn describe_type<W: core::fmt::Write>(mut writer: W) -> core::fmt::Result {
write!(writer, "The log rotation policy")
}
}
Expand Down
2 changes: 1 addition & 1 deletion ballista/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ path = "src/bin/main.rs"
required-features = ["build-binary"]

[features]
build-binary = ["configure_me", "tracing-subscriber", "tracing-appender", "tracing"]
build-binary = ["configure_me", "tracing-subscriber", "tracing-appender", "tracing", "ballista-core/build-binary"]
default = ["build-binary", "mimalloc"]

[dependencies]
Expand Down
10 changes: 4 additions & 6 deletions ballista/scheduler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ path = "src/bin/main.rs"
required-features = ["build-binary"]

[features]
build-binary = ["configure_me", "tracing-subscriber", "tracing-appender", "tracing"]
build-binary = ["configure_me", "clap", "tracing-subscriber", "tracing-appender", "tracing", "ballista-core/build-binary"]
default = ["build-binary"]
flight-sql = []
flight-sql = ["base64"]
keda-scaler = []
prometheus-metrics = ["prometheus", "once_cell"]
rest-api = []
Expand All @@ -47,14 +47,13 @@ arrow-flight = { workspace = true }
async-trait = { workspace = true }
axum = "0.7.7"
ballista-core = { path = "../core", version = "0.12.0" }
base64 = { version = "0.22" }
clap = { workspace = true }
base64 = { version = "0.22", optional = true }
clap = { workspace = true, optional = true }
configure_me = { workspace = true, optional = true }
dashmap = { workspace = true }
datafusion = { workspace = true }
datafusion-proto = { workspace = true }
futures = { workspace = true }
graphviz-rust = "0.9.0"
http = "1.1"
log = { workspace = true }
object_store = { workspace = true }
Expand All @@ -74,7 +73,6 @@ tracing-subscriber = { workspace = true, optional = true }
uuid = { workspace = true }

[dev-dependencies]
ballista-core = { path = "../core", version = "0.12.0" }

[build-dependencies]
configure_me_codegen = { workspace = true }
Expand Down
7 changes: 4 additions & 3 deletions ballista/scheduler/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
use crate::SessionBuilder;
use ballista_core::{config::TaskSchedulingPolicy, ConfigProducer};
use clap::ValueEnum;
use datafusion_proto::logical_plan::LogicalExtensionCodec;
use datafusion_proto::physical_plan::PhysicalExtensionCodec;
use std::sync::Arc;
Expand Down Expand Up @@ -211,7 +210,8 @@ pub enum ClusterStorageConfig {
/// Policy of distributing tasks to available executor slots
///
/// It needs to be visible to code generated by configure_me
#[derive(Clone, ValueEnum, Copy, Debug, serde::Deserialize)]
#[derive(Clone, Copy, Debug, serde::Deserialize)]
#[cfg_attr(feature = "build-binary", derive(clap::ValueEnum))]
pub enum TaskDistribution {
/// Eagerly assign tasks to executor slots. This will assign as many task slots per executor
/// as are currently available
Expand All @@ -226,11 +226,12 @@ pub enum TaskDistribution {
ConsistentHash,
}

#[cfg(feature = "build-binary")]
impl std::str::FromStr for TaskDistribution {
type Err = String;

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
ValueEnum::from_str(s, true)
clap::ValueEnum::from_str(s, true)
}
}

Expand Down
1 change: 0 additions & 1 deletion benchmarks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ homepage = "https://github.com/apache/arrow-ballista"
repository = "https://github.com/apache/arrow-ballista"
license = "Apache-2.0"
publish = false
rust-version = "1.72"

[features]
ci = []
Expand Down
1 change: 0 additions & 1 deletion examples/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ license = "Apache-2.0"
keywords = ["arrow", "distributed", "query", "sql"]
edition = "2021"
publish = false
rust-version = "1.72"

[[example]]
name = "standalone_sql"
Expand Down
1 change: 0 additions & 1 deletion python/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ description = "Apache Arrow Ballista Python Client"
readme = "README.md"
license = "Apache-2.0"
edition = "2021"
rust-version = "1.72"
include = ["/src", "/ballista", "/LICENSE.txt", "pyproject.toml", "Cargo.toml", "Cargo.lock"]
publish = false

Expand Down

0 comments on commit 17cb505

Please sign in to comment.