adding IBEJI_HOME env var for loading the invehicle digital twin config file #61

odra · 2023-10-25T15:19:16Z

Changes

Adds an optional "IBEJI_HOME" env var support to be used when using the invehicle-digital-twin binary

Verification

Running the binary with and without the env var should return the correct error path:

# without (should work as it is)
$ ./target/debug/invehicle-digital-twin 
[2023-10-25T15:13:40Z INFO  invehicle_digital_twin] The In-Vehicle Digital Twin Service has started.
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: configuration file "invehicle_digital_twin_settings" not found', core/invehicle-digital-twin/src/invehicle_digital_twin_config.rs:25:98
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

# using the env var:
$ IBEJI_HOME=/etc/ibeji ./target/debug/invehicle-digital-twin 
[2023-10-25T15:13:53Z INFO  invehicle_digital_twin] The In-Vehicle Digital Twin Service has started.
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: configuration file "/etc/ibeji/invehicle_digital_twin_settings" not found', core/invehicle-digital-twin/src/invehicle_digital_twin_config.rs:25:98
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

…ig file Signed-off-by: Leonardo Rossetti <[email protected]>

devkelley

Thanks for submitting a PR @odra. Please take a look at the minor comments below.

devkelley · 2023-10-26T18:36:55Z

core/invehicle-digital-twin/src/invehicle_digital_twin_config.rs

@@ -15,8 +16,13 @@ pub struct Settings {

 /// Load the settings.
 pub fn load_settings() -> Settings {
+    let config_filename_path = match env::var("IBEJI_HOME") {


Please make IBEJI_HOME a constant in "/core/common/src/utils.rs".

devkelley · 2023-10-26T18:39:00Z

core/invehicle-digital-twin/src/invehicle_digital_twin_config.rs

+    let config_filename_path = match env::var("IBEJI_HOME") {
+        Ok(s) => format!("{}/{}", s, CONFIG_FILENAME),
+        _ => CONFIG_FILENAME.to_owned()
+    };


Please add this IBEJI_HOME matching statement to "/core/common/src/utils.rs" in the load_settings<T>(config_filename: &str) method.

ashbeitz

Just one comment. Please take a look.

ashbeitz · 2023-10-26T19:32:40Z

core/invehicle-digital-twin/src/invehicle_digital_twin_config.rs

@@ -15,8 +16,13 @@ pub struct Settings {

 /// Load the settings.
 pub fn load_settings() -> Settings {
+    let config_filename_path = match env::var("IBEJI_HOME") {


Please provide details on the IBEJI_HOME environment variable in Ibeji's main README.

Signed-off-by: Leonardo Rossetti <[email protected]>

ashbeitz

Comment on your recent changes.

BTW: I kicked off the checks for you. Please take a look at their results.

ashbeitz · 2023-10-27T15:48:41Z

README.md

 `./invehicle-digital-twin`<br>
+Use the `IBEJI_HOME` env var to load configuration files from a specific directory: <br>


Please use "variable" rather than "var". This will also fix the associated spelling check error.

ashbeitz · 2023-10-27T15:55:51Z

core/invehicle-digital-twin/src/invehicle_digital_twin_config.rs

@@ -15,10 +15,7 @@ pub struct Settings {

 /// Load the settings.
 pub fn load_settings() -> Settings {
-    let config =
-        Config::builder().add_source(File::new(CONFIG_FILENAME, FileFormat::Yaml)).build().unwrap();
+    let s = utils::load_settings(CONFIG_FILENAME).unwrap();


Please try to use a meaningful identifier name rather than single character.

We were previously using "settings", rather than "s".

whoops, sorry, forgot to rename it

Signed-off-by: Leonardo Rossetti <[email protected]>

ashbeitz · 2023-10-27T18:18:29Z

README.md

 `./invehicle-digital-twin`<br>
+Use the `IBEJI_HOME` environment variable to load configuration files from a specific directory: <br>


Your change here is for one specific sample. You could discuss this environment variable before the details for each sample. Alternatively, you could copy your change into each sample's details.

I think a high level directive makes more sense and scales better. Let me move it upwards.

Signed-off-by: Leonardo Rossetti <[email protected]>

ashbeitz · 2023-10-27T19:48:07Z

@odra - Please address the issues raised by the checks. You can ignore the "Alex Recommends - language" checks and the "Lint and Check Markdown" checks, as these are caused by known issues. The other checks must be resolved.

ashbeitz · 2023-10-30T16:39:44Z

@odra - Please fix the issues that the "Rust CI / static_code_analysis" check detected. Thanks.

Signed-off-by: Leonardo Rossetti <[email protected]>

odra · 2023-10-30T18:53:16Z

I've sent an update. Would you like for me to squash all my commits into a single one?

ashbeitz · 2023-10-30T19:50:45Z

I've sent an update. Would you like for me to squash all my commits into a single one?

Please do this when you merge in your changes.

I'll kick off the checks again to see if everything looks clean.

ashbeitz · 2023-10-30T19:58:44Z

@odra - The "Rust CI / static_code_analysis" check is still failing. You need to run "cargo fmt" on your PR branch and push the changes.

Signed-off-by: Leonardo Rossetti <[email protected]>

odra · 2023-10-30T20:02:19Z

Just sent a fix :-)

ashbeitz

Approved

ashbeitz · 2023-10-30T21:08:55Z

@odra - Since you do not have committer privileges, I squashed and merged your changes for you.

odra · 2023-10-30T21:40:25Z

No worries, thanks for the review :)

…ig file (eclipse-ibeji#61) * adding IBEJI_HOME env var for loading the invehicle digital twin config file Signed-off-by: Leonardo Rossetti <[email protected]> * use common::utils to load config file Signed-off-by: Leonardo Rossetti <[email protected]> * IBEJI_HOME env var docs Signed-off-by: Leonardo Rossetti <[email protected]> * fix variable name and readme typo Signed-off-by: Leonardo Rossetti <[email protected]> * move environment variable instructions Signed-off-by: Leonardo Rossetti <[email protected]> * adding code block language Signed-off-by: Leonardo Rossetti <[email protected]> * remove unecessary let Signed-off-by: Leonardo Rossetti <[email protected]> * static analysis fix Signed-off-by: Leonardo Rossetti <[email protected]> --------- Signed-off-by: Leonardo Rossetti <[email protected]>

adding IBEJI_HOME env var for loading the invehicle digital twin conf…

82cf4a6

…ig file Signed-off-by: Leonardo Rossetti <[email protected]>

devkelley reviewed Oct 26, 2023

View reviewed changes

ashbeitz reviewed Oct 26, 2023

View reviewed changes

odra added 2 commits October 27, 2023 07:09

use common::utils to load config file

58b86ea

Signed-off-by: Leonardo Rossetti <[email protected]>

IBEJI_HOME env var docs

ce0a9d5

Signed-off-by: Leonardo Rossetti <[email protected]>

ashbeitz reviewed Oct 27, 2023

View reviewed changes

fix variable name and readme typo

ccb8e1e

Signed-off-by: Leonardo Rossetti <[email protected]>

ashbeitz reviewed Oct 27, 2023

View reviewed changes

move environment variable instructions

49c4e9d

Signed-off-by: Leonardo Rossetti <[email protected]>

odra added 2 commits October 30, 2023 15:49

adding code block language

f69f44d

Signed-off-by: Leonardo Rossetti <[email protected]>

remove unecessary let

cce995c

Signed-off-by: Leonardo Rossetti <[email protected]>

static analysis fix

5ded241

Signed-off-by: Leonardo Rossetti <[email protected]>

ashbeitz approved these changes Oct 30, 2023

View reviewed changes

ashbeitz merged commit 27dffdb into eclipse-ibeji:main Oct 30, 2023
9 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding IBEJI_HOME env var for loading the invehicle digital twin config file #61

adding IBEJI_HOME env var for loading the invehicle digital twin config file #61

odra commented Oct 25, 2023

devkelley left a comment

devkelley Oct 26, 2023

devkelley Oct 26, 2023

ashbeitz left a comment

ashbeitz Oct 26, 2023

ashbeitz left a comment

ashbeitz Oct 27, 2023

ashbeitz Oct 27, 2023

ashbeitz Oct 27, 2023

odra Oct 27, 2023

ashbeitz Oct 27, 2023

odra Oct 27, 2023

ashbeitz commented Oct 27, 2023

ashbeitz commented Oct 30, 2023

odra commented Oct 30, 2023

ashbeitz commented Oct 30, 2023

ashbeitz commented Oct 30, 2023

odra commented Oct 30, 2023

ashbeitz left a comment

ashbeitz commented Oct 30, 2023

odra commented Oct 30, 2023

		`./invehicle-digital-twin`<br>
		Use the `IBEJI_HOME` env var to load configuration files from a specific directory: <br>

		`./invehicle-digital-twin`<br>
		Use the `IBEJI_HOME` environment variable to load configuration files from a specific directory: <br>

adding IBEJI_HOME env var for loading the invehicle digital twin config file #61

adding IBEJI_HOME env var for loading the invehicle digital twin config file #61

Conversation

odra commented Oct 25, 2023

Changes

Verification

devkelley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashbeitz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashbeitz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashbeitz commented Oct 27, 2023

ashbeitz commented Oct 30, 2023

odra commented Oct 30, 2023

ashbeitz commented Oct 30, 2023

ashbeitz commented Oct 30, 2023

odra commented Oct 30, 2023

ashbeitz left a comment

Choose a reason for hiding this comment

ashbeitz commented Oct 30, 2023

odra commented Oct 30, 2023