From adc7a28caf0349067d7b43dc34fb7d1802a47c7e Mon Sep 17 00:00:00 2001
From: Nikos Kontakis <wirednkod@gmail.com>
Date: Fri, 15 Sep 2023 17:46:04 +0300
Subject: [PATCH] Address some PR comments
---
crates/configuration/src/global_settings.rs | 3 +-
crates/configuration/src/network.rs | 6 +-
crates/configuration/src/parachain.rs | 2 +-
crates/configuration/src/relaychain.rs | 7 +-
crates/configuration/src/shared/node.rs | 18 +---
crates/configuration/src/shared/resources.rs | 88 +++++++++-----------
crates/configuration/src/shared/types.rs | 81 +++++++++---------
7 files changed, 90 insertions(+), 115 deletions(-)
diff --git a/crates/configuration/src/global_settings.rs b/crates/configuration/src/global_settings.rs
index 24830ac..ca0a0ab 100644
--- a/crates/configuration/src/global_settings.rs
+++ b/crates/configuration/src/global_settings.rs
@@ -12,8 +12,7 @@ use crate::shared::{
/// Global settings applied to an entire network.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct GlobalSettings {
- #[serde(skip_serializing_if = "std::vec::Vec::is_empty")]
- #[serde(default)]
+ #[serde(skip_serializing_if = "std::vec::Vec::is_empty", default)]
bootnodes_addresses: Vec<Multiaddr>,
// TODO: parse both case in zombienet node version to avoid renamed ?
#[serde(rename = "timeout")]
diff --git a/crates/configuration/src/network.rs b/crates/configuration/src/network.rs
index c2a9b83..200452d 100644
--- a/crates/configuration/src/network.rs
+++ b/crates/configuration/src/network.rs
@@ -27,11 +27,9 @@ pub struct NetworkConfig {
#[serde(rename = "settings")]
global_settings: GlobalSettings,
relaychain: Option<RelaychainConfig>,
- #[serde(skip_serializing_if = "std::vec::Vec::is_empty")]
- #[serde(default)]
+ #[serde(skip_serializing_if = "std::vec::Vec::is_empty", default)]
parachains: Vec<ParachainConfig>,
- #[serde(skip_serializing_if = "std::vec::Vec::is_empty")]
- #[serde(default)]
+ #[serde(skip_serializing_if = "std::vec::Vec::is_empty", default)]
hrmp_channels: Vec<HrmpChannelConfig>,
}
diff --git a/crates/configuration/src/parachain.rs b/crates/configuration/src/parachain.rs
index 79c8499..db8db9a 100644
--- a/crates/configuration/src/parachain.rs
+++ b/crates/configuration/src/parachain.rs
@@ -229,7 +229,7 @@ impl<A> ParachainConfigBuilder<A> {
default_image: self.config.default_image.clone(),
default_resources: self.config.default_resources.clone(),
default_db_snapshot: self.config.default_db_snapshot.clone(),
- default_args: self.config.default_args().into_iter().cloned().collect(),
+ default_args: self.config.default_args.clone(),
}
}
}
diff --git a/crates/configuration/src/relaychain.rs b/crates/configuration/src/relaychain.rs
index a66ae59..e761e3c 100644
--- a/crates/configuration/src/relaychain.rs
+++ b/crates/configuration/src/relaychain.rs
@@ -80,7 +80,6 @@ impl RelaychainConfig {
self.nodes.iter().collect::<Vec<&NodeConfig>>()
}
- /// The nodes of the relay chain.
pub(crate) fn set_nodes(&mut self, nodes: Vec<NodeConfig>) {
self.nodes = nodes;
}
@@ -141,11 +140,11 @@ impl<A> RelaychainConfigBuilder<A> {
fn default_chain_context(&self) -> ChainDefaultContext {
ChainDefaultContext {
- default_command: self.config.default_command().cloned(),
- default_image: self.config.default_image().cloned(),
+ default_command: self.config.default_command.clone(),
+ default_image: self.config.default_image.clone(),
default_resources: self.config.default_resources.clone(),
default_db_snapshot: self.config.default_db_snapshot.clone(),
- default_args: self.config.default_args().into_iter().cloned().collect(),
+ default_args: self.config.default_args.clone(),
}
}
}
diff --git a/crates/configuration/src/shared/node.rs b/crates/configuration/src/shared/node.rs
index fe59df2..b380f7e 100644
--- a/crates/configuration/src/shared/node.rs
+++ b/crates/configuration/src/shared/node.rs
@@ -104,14 +104,7 @@ impl Serialize for NodeConfig {
state.serialize_field("command", &self.command)?;
}
- if self.args.is_empty()
- || self.args()
- == self
- .chain_context
- .default_args
- .iter()
- .collect::<Vec<&Arg>>()
- {
+ if self.args.is_empty() || self.args == self.chain_context.default_args {
state.skip_field("args")?;
} else {
state.serialize_field("args", &self.args)?;
@@ -168,20 +161,11 @@ impl NodeConfig {
self.image.as_ref()
}
- /// The default container image used for nodes.
- pub fn set_image(&mut self, image: Image) {
- self.image = Some(image);
- }
-
/// Command to run the node.
pub fn command(&self) -> Option<&Command> {
self.command.as_ref()
}
- pub fn set_command(&mut self, command: Command) {
- self.command = Some(command);
- }
-
/// Arguments to use for node.
pub fn args(&self) -> Vec<&Arg> {
self.args.iter().collect()
diff --git a/crates/configuration/src/shared/resources.rs b/crates/configuration/src/shared/resources.rs
index e1bd469..7b5ac06 100644
--- a/crates/configuration/src/shared/resources.rs
+++ b/crates/configuration/src/shared/resources.rs
@@ -77,6 +77,12 @@ pub struct Resources {
limit_cpu: Option<ResourceQuantity>,
}
+#[derive(Serialize, Deserialize)]
+struct ResourcesField {
+ memory: Option<ResourceQuantity>,
+ cpu: Option<ResourceQuantity>,
+}
+
impl Serialize for Resources {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
@@ -84,12 +90,6 @@ impl Serialize for Resources {
{
let mut state = serializer.serialize_struct("Resources", 2)?;
- #[derive(Serialize)]
- struct ResourcesField {
- memory: Option<ResourceQuantity>,
- cpu: Option<ResourceQuantity>,
- }
-
if self.request_memory.is_some() || self.request_memory.is_some() {
state.serialize_field(
"requests",
@@ -118,54 +118,48 @@ impl Serialize for Resources {
}
}
-impl<'de> Deserialize<'de> for Resources {
- fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
- where
- D: serde::Deserializer<'de>,
- {
- struct ResourcesVisitor;
+struct ResourcesVisitor;
- #[derive(Deserialize)]
- struct ResourcesField {
- memory: Option<ResourceQuantity>,
- cpu: Option<ResourceQuantity>,
- }
+impl<'de> de::Visitor<'de> for ResourcesVisitor {
+ type Value = Resources;
- impl<'de> de::Visitor<'de> for ResourcesVisitor {
- type Value = Resources;
+ fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+ formatter.write_str("a resources object")
+ }
- fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
- formatter.write_str("a resources object")
- }
+ fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
+ where
+ A: de::MapAccess<'de>,
+ {
+ let mut resources: Resources = Resources::default();
- fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
- where
- A: de::MapAccess<'de>,
- {
- let mut resources: Resources = Resources::default();
-
- while let Some((key, value)) = map.next_entry::<String, ResourcesField>()? {
- match key.as_str() {
- "requests" => {
- resources.request_memory = value.memory;
- resources.request_cpu = value.cpu;
- },
- "limits" => {
- resources.limit_memory = value.memory;
- resources.limit_cpu = value.cpu;
- },
- _ => {
- return Err(de::Error::unknown_field(
- &key,
- &["requests", "limits", "cpu", "memory"],
- ))
- },
- }
- }
- Ok(resources)
+ while let Some((key, value)) = map.next_entry::<String, ResourcesField>()? {
+ match key.as_str() {
+ "requests" => {
+ resources.request_memory = value.memory;
+ resources.request_cpu = value.cpu;
+ },
+ "limits" => {
+ resources.limit_memory = value.memory;
+ resources.limit_cpu = value.cpu;
+ },
+ _ => {
+ return Err(de::Error::unknown_field(
+ &key,
+ &["requests", "limits", "cpu", "memory"],
+ ))
+ },
}
}
+ Ok(resources)
+ }
+}
+impl<'de> Deserialize<'de> for Resources {
+ fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+ where
+ D: serde::Deserializer<'de>,
+ {
deserializer.deserialize_any(ResourcesVisitor)
}
}
diff --git a/crates/configuration/src/shared/types.rs b/crates/configuration/src/shared/types.rs
index dd14a2b..46fed58 100644
--- a/crates/configuration/src/shared/types.rs
+++ b/crates/configuration/src/shared/types.rs
@@ -1,4 +1,5 @@
use std::{
+ error::Error,
fmt::{self, Display},
path::PathBuf,
str::FromStr,
@@ -32,13 +33,13 @@ impl From<u128> for U128 {
}
}
-// impl TryFrom<&str> for U128 {
-// type Error = ConversionError;
+impl TryFrom<&str> for U128 {
+ type Error = Box<dyn Error>;
-// fn try_from(value: &str) -> Result<Self, Self::Error> {
-// Ok(Self(value.to_string().parse::<u128>().unwrap()))
-// }
-// }
+ fn try_from(value: &str) -> Result<Self, Self::Error> {
+ Ok(Self(value.to_string().parse::<u128>()?))
+ }
+}
impl Serialize for U128 {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -51,28 +52,28 @@ impl Serialize for U128 {
}
}
+struct U128Visitor;
+
+impl<'de> de::Visitor<'de> for U128Visitor {
+ type Value = U128;
+
+ fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+ formatter.write_str("an integer between 0 and 2^128 − 1.")
+ }
+
+ fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+ where
+ E: de::Error,
+ {
+ v.try_into().map_err(de::Error::custom)
+ }
+}
+
impl<'de> Deserialize<'de> for U128 {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
- struct U128Visitor;
-
- impl<'de> de::Visitor<'de> for U128Visitor {
- type Value = U128;
-
- fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
- formatter.write_str("an integer between 0 and 2^128 − 1.")
- }
-
- fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
- where
- E: de::Error,
- {
- Ok(U128(v.to_string().parse::<u128>().unwrap()))
- }
- }
-
deserializer.deserialize_str(U128Visitor)
}
}
@@ -271,28 +272,28 @@ impl Serialize for AssetLocation {
}
}
+struct AssetLocationVisitor;
+
+impl<'de> de::Visitor<'de> for AssetLocationVisitor {
+ type Value = AssetLocation;
+
+ fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+ formatter.write_str("a string")
+ }
+
+ fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+ where
+ E: de::Error,
+ {
+ Ok(AssetLocation::from(v))
+ }
+}
+
impl<'de> Deserialize<'de> for AssetLocation {
fn deserialize<D>(deserializer: D) -> Result<AssetLocation, D::Error>
where
D: Deserializer<'de>,
{
- struct AssetLocationVisitor;
-
- impl<'de> de::Visitor<'de> for AssetLocationVisitor {
- type Value = AssetLocation;
-
- fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
- formatter.write_str("a string")
- }
-
- fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
- where
- E: de::Error,
- {
- Ok(AssetLocation::from(v))
- }
- }
-
deserializer.deserialize_any(AssetLocationVisitor)
}
}
--
GitLab