Skip to content

Commit

Permalink
fix(commons): do not fail without Vars section
Browse files Browse the repository at this point in the history
If the `Vars` section was not present in a file that is parsed by the
function `try_parse_from_file` then an error would be thrown.

This commit changes the behaviour of the function such that if it is
missing, no error is returned and instead a log message is recorded.

* Cargo.lock: added dependency `tracing`.
* zenoh-flow-commons/Cargo.toml: added dependency `tracing`.

* zenoh-flow-commons/src/utils.rs: instead of throwing an error if the
  `Vars` section is missing, log a debug message and continue the normal
  execution.

Signed-off-by: Julien Loudet <[email protected]>
  • Loading branch information
J-Loudet committed Aug 16, 2024
1 parent be9f4df commit f41262a
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 16 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions zenoh-flow-commons/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ humantime = "2.1"
serde = { workspace = true }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
tracing = { workspace = true }
uuid = { workspace = true }
zenoh-keyexpr = { workspace = true }
zenoh-protocol = { workspace = true }
47 changes: 31 additions & 16 deletions zenoh-flow-commons/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ use serde::Deserialize;

use crate::{IMergeOverwrite, Result, Vars};

/// Given the [Path] of a file, return the function we should call to deserialize an instance of `N`.
/// Given the [Path] of a file, return the function we should call to deserialize an instance of
/// `N`.
///
/// This function will look at the extension of the [Path] to decide on a deserializer.
///
/// # Errors
///
/// This function will fail if the extension of the [Path] is not supported. For now, the only supported extensions are:
/// This function will fail if the extension of the [Path] is not supported. For now, the only
/// supported extensions are:
/// - ".yml"
/// - ".yaml"
/// - ".json"
Expand Down Expand Up @@ -63,25 +65,27 @@ Currently supported file extensions are:
}
}

/// Attempts to parse an instance of `N` from the content of the file located at `path`, overwriting (or complementing)
/// the [Vars] declared in said file with the provided `vars`.
/// Attempts to parse an instance of `N` from the content of the file located at `path`, overwriting
/// (or complementing) the [Vars] declared in said file with the provided `vars`.
///
/// This function is notably used to parse a data flow descriptor. Two file types are supported, identified by their
/// extension:
/// This function is notably used to parse a data flow descriptor. Two file types are supported,
/// identified by their extension:
/// - JSON (`.json` file extension)
/// - YAML (`.yaml` or `.yml` extensions)
///
/// This function does not impose writing *all* descriptor file(s), within the same data flow, in the same format.
/// This function does not impose writing *all* descriptor file(s), within the same data flow, in
/// the same format.
///
/// # Errors
///
/// The parsing can fail for several reasons (listed in sequential order):
/// - the OS failed to [canonicalize](std::fs::canonicalize()) the path of the file,
/// - the OS failed to open (in read mode) the file,
/// - the extension of the file is not supported by Zenoh-Flow (i.e. it's neither a YAML file or a JSON file),
/// - the extension of the file is not supported by Zenoh-Flow (i.e. it's neither a YAML file or a
/// JSON file),
/// - parsing the [Vars] section failed (if there is one),
/// - expanding the variables located in the [Vars] section failed (if there are any) --- see the documentation
/// [handlebars] for a more complete list of reasons,
/// - expanding the variables located in the [Vars] section failed (if there are any) --- see the
/// documentation [handlebars] for a more complete list of reasons,
/// - parsing an instance of `N` failed.
pub fn try_parse_from_file<N>(path: impl AsRef<Path>, vars: Vars) -> Result<(N, Vars)>
where
Expand All @@ -101,18 +105,29 @@ where
path_buf.display()
))?;

let merged_vars = vars.merge_overwrite(
deserializer::<Vars>(&path_buf)?(&buf).context("Failed to deserialize Vars")?,
);
let mut merged_vars = vars;
match deserializer::<Vars>(&path_buf)?(&buf) {
Ok(parsed_vars) => {
merged_vars = merged_vars.merge_overwrite(parsed_vars);
}
Err(e) => {
// NOTE: Maybe the deserialisation fails because there is no `Vars` section. This is not
// necessarily an issue, hence the level "debug" for the log.
tracing::debug!("Could not parse Vars");
tracing::trace!("{e:?}");
tracing::trace!("Maybe the above error is normal as there is no `Vars` section?")
}
}

let mut handlebars = Handlebars::new();
handlebars.set_strict_mode(true);

let rendered_descriptor = handlebars
// NOTE: We have to dereference `merged_vars` (this: `&(*merged_vars)`) and pass the contained `HashMap` such
// that `handlebars` can correctly manipulate it.
// NOTE: We have to dereference `merged_vars` (this: `&(*merged_vars)`) and pass the
// contained `HashMap` such that `handlebars` can correctly manipulate it.
//
// We have to have this indirection in the structure such that `serde` can correctly deserialise the descriptor.
// We have to have this indirection in the structure such that `serde` can correctly
// deserialise the descriptor.
.render_template(buf.as_str(), &(*merged_vars))
.context("Failed to expand descriptor")?;

Expand Down

0 comments on commit f41262a

Please sign in to comment.