[Breaking] Remove backwards compatibility for configs (#1695)

This was previously deprecated in v0.5.4.

Author: jhpratt

nativelink-config/BUILD.bazel

@@ -32,7 +32,6 @@ rust_test_suite(
     name = "integration",
     timeout = "short",
     srcs = [
-        "tests/backwards_compat_test.rs",
         "tests/deserialization_test.rs",
     ],
     deps = [

nativelink-config/src/cas_server.rs

@@ -16,14 +16,14 @@ use std::collections::HashMap;
 
 use serde::Deserialize;
 
+use crate::schedulers::SchedulerSpec;
 use crate::serde_utils::{
     convert_data_size_with_shellexpand, convert_duration_with_shellexpand,
     convert_numeric_with_shellexpand, convert_optional_numeric_with_shellexpand,
     convert_optional_string_with_shellexpand, convert_string_with_shellexpand,
     convert_vec_string_with_shellexpand,
 };
-use crate::stores::{ClientTlsConfig, ConfigDigestHashFunction, StoreRefName};
-use crate::{SchedulerConfigs, StoreConfigs};
+use crate::stores::{ClientTlsConfig, ConfigDigestHashFunction, StoreRefName, StoreSpec};
 
 /// Name of the scheduler. This type will be used when referencing a
 /// scheduler in the `CasConfig::schedulers`'s map key.
@@ -716,20 +716,30 @@ pub struct GlobalConfig {
     pub default_digest_size_health_check: usize,
 }
 
+#[derive(Debug, Clone, Deserialize)]
+pub struct NamedConfig<Spec> {
+    pub name: String,
+    #[serde(flatten)]
+    pub spec: Spec,
+}
+
+pub type StoreConfig = NamedConfig<StoreSpec>;
+pub type SchedulerConfig = NamedConfig<SchedulerSpec>;
+
 #[derive(Deserialize, Debug)]
 #[serde(deny_unknown_fields)]
 pub struct CasConfig {
     /// List of stores available to use in this config.
     /// The keys can be used in other configs when needing to reference a store.
-    pub stores: StoreConfigs,
+    pub stores: Vec<StoreConfig>,
 
     /// Worker configurations used to execute jobs.
     pub workers: Option<Vec<WorkerConfig>>,
 
     /// List of schedulers available to use in this config.
     /// The keys can be used in other configs when needing to reference a
     /// scheduler.
-    pub schedulers: Option<SchedulerConfigs>,
+    pub schedulers: Option<Vec<SchedulerConfig>>,
 
     /// Servers to setup for this process.
     pub servers: Vec<ServerConfig>,

nativelink-config/src/lib.rs

@@ -16,147 +16,3 @@ pub mod cas_server;
 pub mod schedulers;
 pub mod serde_utils;
 pub mod stores;
-
-use std::any::type_name;
-use std::collections::HashMap;
-use std::fmt;
-use std::marker::PhantomData;
-
-use serde::de::{MapAccess, SeqAccess, Visitor};
-use serde::{Deserialize, Deserializer};
-
-#[derive(Debug, Clone, Deserialize)]
-pub struct NamedConfig<Spec> {
-    pub name: String,
-    #[serde(flatten)]
-    pub spec: Spec,
-}
-
-pub type StoreConfig = NamedConfig<stores::StoreSpec>;
-pub type SchedulerConfig = NamedConfig<schedulers::SchedulerSpec>;
-
-// TODO(aaronmondal): Remove all the iterator impls and the Deserializer once we
-//                    fully migrate to the new config schema.
-pub type StoreConfigs = NamedConfigs<stores::StoreSpec>;
-pub type SchedulerConfigs = NamedConfigs<schedulers::SchedulerSpec>;
-
-#[derive(Debug)]
-pub struct NamedConfigs<T>(pub Vec<NamedConfig<T>>);
-
-impl<T> NamedConfigs<T> {
-    pub fn iter(&self) -> std::slice::Iter<'_, NamedConfig<T>> {
-        self.0.iter()
-    }
-}
-
-impl<T> IntoIterator for NamedConfigs<T> {
-    type Item = NamedConfig<T>;
-    type IntoIter = std::vec::IntoIter<Self::Item>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.0.into_iter()
-    }
-}
-
-impl<'a, T> IntoIterator for &'a NamedConfigs<T> {
-    type Item = &'a NamedConfig<T>;
-    type IntoIter = std::slice::Iter<'a, NamedConfig<T>>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.0.iter()
-    }
-}
-
-struct NamedConfigsVisitor<T> {
-    phantom: PhantomData<T>,
-}
-
-impl<T> NamedConfigsVisitor<T> {
-    const fn new() -> Self {
-        NamedConfigsVisitor {
-            phantom: PhantomData,
-        }
-    }
-}
-
-impl<'de, T: Deserialize<'de>> Visitor<'de> for NamedConfigsVisitor<T> {
-    type Value = NamedConfigs<T>;
-
-    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-        formatter.write_str("a sequence or map of named configs")
-    }
-
-    fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
-    where
-        A: SeqAccess<'de>,
-    {
-        let mut vec = Vec::new();
-        while let Some(config) = seq.next_element()? {
-            vec.push(config);
-        }
-        Ok(NamedConfigs(vec))
-    }
-
-    fn visit_map<M>(self, mut access: M) -> Result<Self::Value, M::Error>
-    where
-        M: MapAccess<'de>,
-    {
-        let config_type = if type_name::<T>().contains("StoreSpec") {
-            "stores"
-        } else if type_name::<T>().contains("SchedulerSpec") {
-            "schedulers"
-        } else {
-            "stores and schedulers"
-        };
-        eprintln!(
-            r#"
-WARNING: Using deprecated map format for {config_type}. Please migrate to the new array format:
-
-  // Old:
-  "stores": {{
-    "SOMESTORE": {{
-      "memory": {{}}
-    }}
-  }},
-  "schedulers": {{
-    "SOMESCHEDULER": {{
-      "simple": {{}}
-    }}
-  }}
-
-  // New:
-  "stores": [
-    {{
-      "name": "SOMESTORE",
-      "memory": {{}}
-    }}
-  ],
-  "schedulers": [
-    {{
-      "name": "SOMESCHEDULER",
-      "simple": {{}}
-    }}
-  ]
-"#
-        );
-
-        let mut map = HashMap::new();
-        while let Some((key, value)) = access.next_entry()? {
-            map.insert(key, value);
-        }
-        Ok(NamedConfigs(
-            map.into_iter()
-                .map(|(name, spec)| NamedConfig { name, spec })
-                .collect(),
-        ))
-    }
-}
-
-impl<'de, T: Deserialize<'de>> Deserialize<'de> for NamedConfigs<T> {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        deserializer.deserialize_any(NamedConfigsVisitor::new())
-    }
-}

nativelink-config/tests/backwards_compat_test.rs

@@ -28,10 +28,10 @@ use hyper_util::server::conn::auto;
 use hyper_util::service::TowerToHyperService;
 use mimalloc::MiMalloc;
 use nativelink_config::cas_server::{
-    CasConfig, GlobalConfig, HttpCompressionAlgorithm, ListenerConfig, ServerConfig, WorkerConfig,
+    CasConfig, GlobalConfig, HttpCompressionAlgorithm, ListenerConfig, SchedulerConfig,
+    ServerConfig, StoreConfig, WorkerConfig,
 };
 use nativelink_config::stores::ConfigDigestHashFunction;
-use nativelink_config::{SchedulerConfig, StoreConfig};
 use nativelink_error::{Code, Error, ResultExt, make_err, make_input_err};
 use nativelink_metric::{
     MetricFieldData, MetricKind, MetricPublishKnownKindData, MetricsComponent, RootMetricsComponent,