Open mversic opened 1 month ago
I would like to be assigned with this issue to get started with it. Also, can you give me some idea about what sort of paramters are required to be set through CLI?
@divyaranjan1905 hi, this is the instruction that's used for setting chain-wide parameters:
@nxsaken Tried looking into surrounding crates to get an idea. Can we make this available to CLI by setting:
#[cfg(feature = "transparent_api")]
On top of this, I guess we also need to import the struct SetParameter
somewhere else to use it? Or, does the model_single!
macro take care of it?
Let me know where I'm off 🙏
@divyaranjan1905 you need to add a subcommand to iroha_client_cli
that executes SetParameter
. Here's a PR that you might use as a reference: https://github.com/hyperledger/iroha/pull/4408 (or you can check the git history to see how others have approached it).
@nxsaken Thank you for the reference. From what I understand from there, after adding Parameters
to enum SubCommand
I defined a mod param
that would contain all the stuff.
What I'm doubtful of is that, do I need to create a struct
that contains all the parameters defined in:
https://github.com/hyperledger/iroha/blob/d8e65f0ef31a8b20a70e840b859b64bc9ae96846/data_model/src/parameter.rs#L30
The commit you mentioned does this in a struct Burn
where they take in the necessary id
, key
etc.
@mversic how should a parameter be parsed from a CLI command?
@a-zorina can you give us an idea on this?
Can we try going further with this?
Let's consider this. The Parameter
enum contains all possible parameter variants that can be set:
Each of the variants can also subdivide into further variants, for example SumeragiParameter
:
There is also CustomParameter
, which allows using user-defined parameters:
We could mirror this hierarchy as CLI subcommands and arguments:
iroha set sumeragi block_time_ms 2000
iroha set block max_transactions 512
iroha set custom --id my_param --payload "{\"key\":\"value\"}"
@mversic @a-zorina thoughts?
let's talk in terms of how using iroha
cli would look like. We have to have both set and get parameter operations.
We could have something like:
iroha parameter set sumeragi block_time_ms 2000
iroha parameter get block max_transactions
iroha parameter set my_param "{\"key\":\"value\"}"
iroha parameter get all
I don't think that we should differentiate between built-in and custom parameters, but I'm not completely sure if that's technically feasible
I think it should be possible to define dynamic subcommands with Clap, the builder API may be helpful here, haven't tried that myself. If it's too messy, it's better to leave it and focus on the common case.
@divyaranjan1905 I think this should unblock you, don't hesitate to reach out if you get stuck or anything.
We could mirror this hierarchy as CLI subcommands and arguments:
iroha set sumeragi block_time_ms 2000 iroha set block max_transactions 512 iroha set custom --id my_param --payload "{\"key\":\"value\"}"
@nxsaken I was thinking along similar lines. I had defined a enum
(say, Set
) that stores the particular parameter that has been provided by the user through CLI, that they want to set. The enum
looks vaguely like this:
pub enum Set {
Sumeragi(SumeragiParameter),
Block(BlockParameter),
}
And then I can have an impl
of this Set
that can match
for the particular parameter that has been selected by enum Set
and do the necessary setting. And for that we use SetParameter
from isi.rs
:
Does this look fine? Now that @mversic mentioned about also getting existing parameters, I guess this enum
needs to be renamed and the operation might change a bit, but does the overall idea seem right?
Moreover, I had a question regarding the CLI usage:
In parameters.rs
there's a struct
for multiple parameters like SumeragiParameters
and an enum
for single parameter like SumeragiParameter
. The above enum Set
only takes the latter, but which one are to be referred to? Or, both of them must be implemented?
Yeah, you'd probably need both Get
and Set
. They are almost the same, except Get
has the Get::All
option, while Set
has a value argument for each parameter. Something like the following:
#[derive(clap::Subcommand)
enum Parameter {
Get(parameter::Get),
Set(iroha_data_model::Parameter),
}
mod parameter {
#[derive(clap::Subcommand)
enum Get {
All,
#[command(flatten)]
Inner(iroha_data_model::ParameterNoDataFields), // this doesn't exist
}
}
You should probably implement some clap-related parsing for iroha_data_model::Parameter
to be compatible with the command to avoid code duplication. On a second thought, iroha_data_model::Parameter
doesn't have a version without data fields, so it could make sense to repeat the whole hierarchy in the CLI – or add a data-less version of the hierarchy to iroha_data_model
and reuse it here.
Ignore SumeragiParameters
, it's not a Parameter
itself.
On a second thought,
iroha_data_model::Parameter
doesn't have a version without data fields
@nxsaken I don't think I understand this, iroha_data_model::Parameter
is an enum with variants referring to other enums themselves that contain the parameter(s). Why would we need something without data fields (variants?)?
@divyaranjan1905 so that the user can specify which parameter they want to read (there is no value to provide)
Okay, got it. I would need to look deeper into clap
and probably try some traits to take care of the parsing.
Here's something weird happening, that either of you might have a clue about @nxsaken @mversic. I am creating an enum
labelling different subcommands and their arguments, as below:
#[derive(Subcommand, Debug)]
pub enum ParamCmd {
#[command(about = "Setting Sumeragi Parameters.")]
Sumeragi(SumeragiCmd),
#[command(about = "Setting Block Parameters.")]
Block(BlockCmd),
#[command(about = "Setting Transaction Parameters.")]
Transaction(TransactionCmd),
#[command(about = "Setting Smart Contract Parameters.")]
SmartContract(SmartContractCmd),
#[command(about = "Setting Custom Parameters.")]
Custom(CustomCmd),
}
For each one of these, I have implemented another enum
as below:
#[derive(Subcommand, Debug)]
enum SumeragiCmd {
#[command(about = "Set block time in milliseconds.")]
BlockTime {
#[arg(long = "block-time" )]
value: u64,
},
#[command(about = "Set commit time in milliseconds.")]
CommitTime {
#[arg(long = "commit-time")]
value: u64,
}
}
With this we now have subcommands, for selecting sumeragi parameter and then choosing which parameter to get/set. The problem is that for some weird reason, I am getting the following error:
rustc [E0277]: error[E0277]: the trait bound `SumeragiCmd: clap::Args` is not satisfied
--> client_cli/src/main.rs:1090:18
|
1090 | Sumeragi(SumeragiCmd),
| ^^^^^^^^^^^ the trait `clap::Args` is not implemented for `SumeragiCmd`
|
= help: the following other types implement trait `clap::Args`:
(dyn clap::Args + 'static)
Box<T>
Get
GetKeyValue
ListPermissions
MetadataArgs
MetadataValueArg
ParamCli
and 19 others
I was wondering if this would be a clap
issue, but before reporting to them I copied the above into a cargo new
one, but the error vanishes in the new crate. Is this to do with the internal types of client_cli
that use clap::Args
trait?
Also, I get the following error when importing use clap::{Parser, Args, Subcommand}
.
rustc [E0603]: the trait import `Args` is defined here...
In SumeragiCmd
, I don't think you need the #[command]
attribute on the variants. Or you can do something similar to MetadataArgs
: derive clap::Args
for it, then use it with #[command(flatten)]
.
Importing Args
might clash with the Args
structs present in main.rs
.
If you still have the error, could you push the code, and maybe even open a draft PR? It would be easier to diagnose issues that way and exchange feedback.
Okay, sure. I'll open a draft PR as soon as I can get the overall ideas implemented.
Or you can do something similar to
MetadataArgs
: deriveclap::Args
for it, then use it with#[command(flatten)]
.
The thing is, clap::Args
doesn't work with enums
, only with structs
.
I should've specified what "it" is. What I'm saying is make a struct ParameterValue { value: ... }
, where ParameterValue
is clap::Args
, and value
has #[arg]
, then reuse it across the enum variants. The enum variant itself gets #[command(flatten)]
. Otherwise, when having the fields directly inside the variant, the variant doesn't need any attribute.
We should add support to set parameters through
iroha
CLI