Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/cli integration #55

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

Conversation

conor-sonalake
Copy link
Contributor

Altered main.rs to add derive api macro and clap macro, in order to prep for integrating clap.

@frankhorv frankhorv marked this pull request as draft June 30, 2022 14:34
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@frankhorv
Copy link
Collaborator

frankhorv commented Jul 1, 2022

Add Default derive to Settings, ApplicationSettings and DatabaseSettings

Then you will be able to create a default instance like

use snmp_sim::configuration::Settings;
let settings = Settings::default();

But to have some meaningful configuration values, you need to set serde::default for each struct fields.
For example, the DatabaseSettings struct and connection_uri field could look like:

#[derive(serde::Deserialize, Clone, Default)]
/// Structure representing the database settings
pub struct DatabaseSettings {
    /// database connection URI
    #[serde(default = "default_connection_uri")]
    connection_uri: String,
    #[serde(default = "default_tests_skip_drop")]
    pub tests_skip_drop: bool,
}

fn default_connection_uri() -> String {
    "sqlite://~/.snmp-sim/snmp-sim.db".to_string()
}

You can consider the base.yaml as default values.
How to serialize a structure into a yaml file https://docs.rs/serde_yaml/latest/serde_yaml/

src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated
fo,
};
}
Err(err) => panic!("This file already exists {:?}", err);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use panic! in production, but propagate the error instead.
The error should inform user, that the file already exists, and if it is intended to be overwritten, then the y argument needs to be involved.

src/cli/cli.rs Outdated Show resolved Hide resolved
src/cli/cli.rs Outdated
}
Err(err) => panic!("This file already exists {:?}", err);
}
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing the write to file part of the code

src/cli/cli.rs Outdated
use std::io::ErrorKind;
use std::path::Path;

// fn write_data_to_file() -> Result<(), serde::yaml::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the commented code

src/main.rs Outdated
use snmp_sim::app::Service;
use snmp_sim::configuration::get_configuration;

#[derive(Parser, Debug)]
#[clap(about, version, author)]
struct Value {/* To be filled with args/values */}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implement the SNMP Simulator service CLI

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and handle the CLI from ./src/main.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants