Skip to content

Commit

Permalink
[Breaking]Standardize configurations so they are all lower case
Browse files Browse the repository at this point in the history
To make the configuration more standardized we have moved all
config properties that might be set in the json to be lower case.

Before we had a mix of sometimes lower case and sometimes upper.
By making this a hard rule, we simplify a lot of this thought.
  • Loading branch information
allada committed Dec 8, 2023
1 parent 4eab282 commit a0ba2c8
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 108 deletions.
23 changes: 13 additions & 10 deletions native-link-config/src/cas_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ pub type SchedulerRefName = String;
/// Used when the config references `instance_name` in the protocol.
pub type InstanceName = String;

#[allow(non_camel_case_types)]
#[derive(Deserialize, Debug, Default, Clone, Copy)]
pub enum CompressionAlgorithm {
/// No compression.
#[default]
None,
none,
/// Zlib compression.
Gzip,
gzip,
}

/// Note: Compressing data in the cloud rarely has a benefit, since most
Expand Down Expand Up @@ -309,33 +310,35 @@ pub struct EndpointConfig {
pub timeout: Option<f32>,
}

#[allow(non_camel_case_types)]
#[derive(Copy, Clone, Deserialize, Debug, Default)]
pub enum UploadCacheResultsStrategy {
/// Only upload action results with an exit code of 0.
#[default]
SuccessOnly,
success_only,

/// Don't upload any action results.
Never,
never,

/// Upload all action results that complete.
Everything,
everything,

/// Only upload action results that fail.
FailuresOnly,
failures_only,
}

#[allow(non_camel_case_types)]
#[derive(Clone, Deserialize, Debug)]
pub enum EnvironmentSource {
/// The name of the property in the action to get the value from.
Property(String),
property(String),

/// The raw value to set.
Value(#[serde(deserialize_with = "convert_string_with_shellexpand")] String),
value(#[serde(deserialize_with = "convert_string_with_shellexpand")] String),

/// The max amount of time in milliseconds the command is allowed to run
/// (requested by the client).
TimeoutMillis,
timeout_millis,

/// A special file path will be provided that can be used to comminicate
/// with the parent process about out-of-band information. This file
Expand All @@ -353,7 +356,7 @@ pub enum EnvironmentSource {
///
/// All fields are optional, file does not need to be created and may be
/// empty.
SideChannelFile,
side_channel_file,
}

#[derive(Deserialize, Debug, Default)]
Expand Down
16 changes: 9 additions & 7 deletions native-link-config/src/schedulers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,26 @@ pub enum SchedulerConfig {

/// When the scheduler matches tasks to workers that are capable of running
/// the task, this value will be used to determine how the property is treated.
#[allow(non_camel_case_types)]
#[derive(Deserialize, Debug, Clone, Copy, Hash, Eq, PartialEq)]
pub enum PropertyType {
/// Requires the platform property to be a u64 and when the scheduler looks
/// for appropriate worker nodes that are capable of executing the task,
/// the task will not run on a node that has less than this value.
Minimum,
minimum,

/// Requires the platform property to be a string and when the scheduler
/// looks for appropriate worker nodes that are capable of executing the
/// task, the task will not run on a node that does not have this property
/// set to the value with exact string match.
Exact,
exact,

/// Does not restrict on this value and instead will be passed to the worker
/// as an informational piece.
/// TODO(allada) In the future this will be used by the scheduler and worker
/// to cause the scheduler to prefer certain workers over others, but not
/// restrict them based on these values.
Priority,
priority,
}

/// When a worker is being searched for to run a job, this will be used
Expand All @@ -59,9 +60,9 @@ pub enum PropertyType {
pub enum WorkerAllocationStrategy {
/// Prefer workers that have been least recently used to run a job.
#[default]
LeastRecentlyUsed,
least_recently_used,
/// Prefer workers that have been most recently used to run a job.
MostRecentlyUsed,
most_recently_used,
}

#[derive(Deserialize, Debug, Default)]
Expand Down Expand Up @@ -156,12 +157,13 @@ pub struct PlatformPropertyAddition {
pub value: String,
}

#[allow(non_camel_case_types)]
#[derive(Deserialize, Debug, Clone)]
pub enum PropertyModification {
/// Add a property to the action properties.
Add(PlatformPropertyAddition),
add(PlatformPropertyAddition),
/// Remove a named property from the action.
Remove(String),
remove(String),
}

#[derive(Deserialize, Debug)]
Expand Down
8 changes: 5 additions & 3 deletions native-link-config/src/stores.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ pub struct Lz4Config {
pub max_decode_block_size: u32,
}

#[allow(non_camel_case_types)]
#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
pub enum CompressionAlgorithm {
/// LZ4 compression algorithm is extremely fast for compression and
Expand All @@ -362,7 +363,7 @@ pub enum CompressionAlgorithm {
/// compressible.
///
/// see: <https://lz4.github.io/lz4/>
LZ4(Lz4Config),
lz4(Lz4Config),
}

#[derive(Serialize, Deserialize, Debug, Clone)]
Expand Down Expand Up @@ -437,12 +438,13 @@ pub struct S3Store {
pub insecure_allow_http: bool,
}

#[allow(non_camel_case_types)]
#[derive(Serialize, Deserialize, Debug, Clone, Copy)]
pub enum StoreType {
/// The store is content addressable storage.
CAS,
cas,
/// The store is an action cache.
AC,
ac,
}

#[derive(Serialize, Deserialize, Debug, Clone)]
Expand Down
2 changes: 1 addition & 1 deletion native-link-scheduler/src/grpc_scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl ActionScheduler for GrpcScheduler {
.err_tip(|| "Unable to get execution properties in GrpcScheduler")?
.supported_node_properties
.iter()
.map(|property| (property.clone(), native_link_config::schedulers::PropertyType::Exact))
.map(|property| (property.clone(), native_link_config::schedulers::PropertyType::exact))
.collect(),
));

Expand Down
6 changes: 3 additions & 3 deletions native-link-scheduler/src/platform_property_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,16 @@ impl PlatformPropertyManager {
pub fn make_prop_value(&self, key: &str, value: &str) -> Result<PlatformPropertyValue, Error> {
if let Some(prop_type) = self.known_properties.get(key) {
return match prop_type {
PropertyType::Minimum => Ok(PlatformPropertyValue::Minimum(value.parse::<u64>().err_tip_with_code(
PropertyType::minimum => Ok(PlatformPropertyValue::Minimum(value.parse::<u64>().err_tip_with_code(
|e| {
(
Code::InvalidArgument,
format!("Cannot convert to platform property to u64: {value} - {e}"),
)
},
)?)),
PropertyType::Exact => Ok(PlatformPropertyValue::Exact(value.to_string())),
PropertyType::Priority => Ok(PlatformPropertyValue::Priority(value.to_string())),
PropertyType::exact => Ok(PlatformPropertyValue::Exact(value.to_string())),
PropertyType::priority => Ok(PlatformPropertyValue::Priority(value.to_string())),
};
}
Err(make_input_err!("Unknown platform property '{}'", key))
Expand Down
10 changes: 5 additions & 5 deletions native-link-scheduler/src/property_modifier_scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ impl ActionScheduler for PropertyModifierScheduler {
let mut known_properties = property_manager.get_known_properties().clone();
for modification in &self.modifications {
match modification {
PropertyModification::Remove(name) => {
known_properties.entry(name.into()).or_insert(PropertyType::Priority);
PropertyModification::remove(name) => {
known_properties.entry(name.into()).or_insert(PropertyType::priority);
}
PropertyModification::Add(_) => (),
PropertyModification::add(_) => (),
}
}
let property_manager = {
Expand All @@ -86,13 +86,13 @@ impl ActionScheduler for PropertyModifierScheduler {
.err_tip(|| "In PropertyModifierScheduler::add_action")?;
for modification in &self.modifications {
match modification {
PropertyModification::Add(addition) => action_info.platform_properties.properties.insert(
PropertyModification::add(addition) => action_info.platform_properties.properties.insert(
addition.name.clone(),
platform_property_manager
.make_prop_value(&addition.name, &addition.value)
.err_tip(|| "In PropertyModifierScheduler::add_action")?,
),
PropertyModification::Remove(name) => action_info.platform_properties.properties.remove(name),
PropertyModification::remove(name) => action_info.platform_properties.properties.remove(name),
};
}
self.scheduler.add_action(action_info).await
Expand Down
4 changes: 2 additions & 2 deletions native-link-scheduler/src/simple_scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ impl Workers {
let mut workers_iter = self.workers.iter_mut();
let workers_iter = match self.allocation_strategy {
// Use rfind to get the least recently used that satisfies the properties.
WorkerAllocationStrategy::LeastRecentlyUsed => workers_iter
WorkerAllocationStrategy::least_recently_used => workers_iter
.rfind(|(_, w)| w.can_accept_work() && action_properties.is_satisfied_by(&w.platform_properties)),
// Use find to get the most recently used that satisfies the properties.
WorkerAllocationStrategy::MostRecentlyUsed => workers_iter
WorkerAllocationStrategy::most_recently_used => workers_iter
.find(|(_, w)| w.can_accept_work() && action_properties.is_satisfied_by(&w.platform_properties)),
};
let worker_id = workers_iter.map(|(_, w)| &w.id);
Expand Down
32 changes: 16 additions & 16 deletions native-link-scheduler/tests/property_modifier_scheduler_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ mod property_modifier_scheduler_tests {
async fn add_action_adds_property() -> Result<(), Error> {
let name = "name".to_string();
let value = "value".to_string();
let context = make_modifier_scheduler(vec![PropertyModification::Add(PlatformPropertyAddition {
let context = make_modifier_scheduler(vec![PropertyModification::add(PlatformPropertyAddition {
name: name.clone(),
value: value.clone(),
})]);
Expand All @@ -76,7 +76,7 @@ mod property_modifier_scheduler_tests {
}));
let platform_property_manager = Arc::new(PlatformPropertyManager::new(HashMap::from([(
name.clone(),
PropertyType::Exact,
PropertyType::exact,
)])));
let (_, _, action_info) = join!(
context.modifier_scheduler.add_action(action_info),
Expand All @@ -97,7 +97,7 @@ mod property_modifier_scheduler_tests {
let name = "name".to_string();
let original_value = "value".to_string();
let replaced_value = "replaced".to_string();
let context = make_modifier_scheduler(vec![PropertyModification::Add(PlatformPropertyAddition {
let context = make_modifier_scheduler(vec![PropertyModification::add(PlatformPropertyAddition {
name: name.clone(),
value: replaced_value.clone(),
})]);
Expand All @@ -112,7 +112,7 @@ mod property_modifier_scheduler_tests {
}));
let platform_property_manager = Arc::new(PlatformPropertyManager::new(HashMap::from([(
name.clone(),
PropertyType::Exact,
PropertyType::exact,
)])));
let (_, _, action_info) = join!(
context.modifier_scheduler.add_action(action_info),
Expand All @@ -133,8 +133,8 @@ mod property_modifier_scheduler_tests {
let name = "name".to_string();
let value = "value".to_string();
let context = make_modifier_scheduler(vec![
PropertyModification::Remove(name.clone()),
PropertyModification::Add(PlatformPropertyAddition {
PropertyModification::remove(name.clone()),
PropertyModification::add(PlatformPropertyAddition {
name: name.clone(),
value: value.clone(),
}),
Expand All @@ -146,7 +146,7 @@ mod property_modifier_scheduler_tests {
}));
let platform_property_manager = Arc::new(PlatformPropertyManager::new(HashMap::from([(
name.clone(),
PropertyType::Exact,
PropertyType::exact,
)])));
let (_, _, action_info) = join!(
context.modifier_scheduler.add_action(action_info),
Expand All @@ -167,11 +167,11 @@ mod property_modifier_scheduler_tests {
let name = "name".to_string();
let value = "value".to_string();
let context = make_modifier_scheduler(vec![
PropertyModification::Add(PlatformPropertyAddition {
PropertyModification::add(PlatformPropertyAddition {
name: name.clone(),
value: value.clone(),
}),
PropertyModification::Remove(name.clone()),
PropertyModification::remove(name.clone()),
]);
let action_info = make_base_action_info(UNIX_EPOCH);
let (_forward_watch_channel_tx, forward_watch_channel_rx) = watch::channel(Arc::new(ActionState {
Expand All @@ -180,7 +180,7 @@ mod property_modifier_scheduler_tests {
}));
let platform_property_manager = Arc::new(PlatformPropertyManager::new(HashMap::from([(
name,
PropertyType::Exact,
PropertyType::exact,
)])));
let (_, _, action_info) = join!(
context.modifier_scheduler.add_action(action_info),
Expand All @@ -197,7 +197,7 @@ mod property_modifier_scheduler_tests {
async fn add_action_property_remove() -> Result<(), Error> {
let name = "name".to_string();
let value = "value".to_string();
let context = make_modifier_scheduler(vec![PropertyModification::Remove(name.clone())]);
let context = make_modifier_scheduler(vec![PropertyModification::remove(name.clone())]);
let mut action_info = make_base_action_info(UNIX_EPOCH);
action_info
.platform_properties
Expand Down Expand Up @@ -239,15 +239,15 @@ mod property_modifier_scheduler_tests {
#[tokio::test]
async fn remove_adds_to_underlying_manager() -> Result<(), Error> {
let name = "name".to_string();
let context = make_modifier_scheduler(vec![PropertyModification::Remove(name.clone())]);
let context = make_modifier_scheduler(vec![PropertyModification::remove(name.clone())]);
let scheduler_property_manager = Arc::new(PlatformPropertyManager::new(HashMap::new()));
let get_property_manager_fut = context
.mock_scheduler
.expect_get_platform_property_manager(Ok(scheduler_property_manager));
let property_manager_fut = context.modifier_scheduler.get_platform_property_manager(INSTANCE_NAME);
let (actual_instance_name, property_manager) = join!(get_property_manager_fut, property_manager_fut);
assert_eq!(
HashMap::<_, _>::from_iter([(name, PropertyType::Priority)]),
HashMap::<_, _>::from_iter([(name, PropertyType::priority)]),
*property_manager?.get_known_properties()
);
assert_eq!(actual_instance_name, INSTANCE_NAME);
Expand All @@ -257,18 +257,18 @@ mod property_modifier_scheduler_tests {
#[tokio::test]
async fn remove_retains_type_in_underlying_manager() -> Result<(), Error> {
let name = "name".to_string();
let context = make_modifier_scheduler(vec![PropertyModification::Remove(name.clone())]);
let context = make_modifier_scheduler(vec![PropertyModification::remove(name.clone())]);
let scheduler_property_manager = Arc::new(PlatformPropertyManager::new(HashMap::<_, _>::from_iter([(
name.clone(),
PropertyType::Exact,
PropertyType::exact,
)])));
let get_property_manager_fut = context
.mock_scheduler
.expect_get_platform_property_manager(Ok(scheduler_property_manager));
let property_manager_fut = context.modifier_scheduler.get_platform_property_manager(INSTANCE_NAME);
let (_, property_manager) = join!(get_property_manager_fut, property_manager_fut);
assert_eq!(
HashMap::<_, _>::from_iter([(name, PropertyType::Exact)]),
HashMap::<_, _>::from_iter([(name, PropertyType::exact)]),
*property_manager?.get_known_properties()
);
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion native-link-store/src/compression_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl CompressionStore {
inner_store: Arc<dyn Store>,
) -> Result<Self, Error> {
let lz4_config = match compression_config.compression_algorithm {
native_link_config::stores::CompressionAlgorithm::LZ4(mut lz4_config) => {
native_link_config::stores::CompressionAlgorithm::lz4(mut lz4_config) => {
if lz4_config.block_size == 0 {
lz4_config.block_size = DEFAULT_BLOCK_SIZE;
}
Expand Down
Loading

0 comments on commit a0ba2c8

Please sign in to comment.