Feature/clap #179
Feature/clap #179
Conversation
heemankv
force-pushed
the
feature/clap
branch
from
c7e5522
to
0456ac3
Compare
heemankv
force-pushed
the
feature/clap
branch
from
0456ac3
to
7529986
Compare
| @@ -17,7 +17,7 @@ AWS_DEFAULT_REGION="localhost" | |||
| ##### STORAGE ##### | |||
|
|
|||
| DATA_STORAGE= | |||
There was a problem hiding this comment.
these ones can also be prefixed with MADARA_ORCHESTRATOR right? I think AWS are the only ones where we can skip because those have a meaning outside orchestrator as well
There was a problem hiding this comment.
We are using cli based booleans to detect among the various XYZProviders to choose from,
i.e --aws_s3 for Storage, so in the current impl there's no way for us to provide these selections from env,
this is done in favour of CLI, do you think it's worth implementing from env as well ? @apoorvsadana
#[derive(Debug, Clone, Args)]
#[group()]
pub struct AWSS3CliArgs {
/// Use the AWS s3 client
#[arg(long)]
pub aws_s3: bool,
/// The name of the S3 bucket.
#[arg(env = "MADARA_ORCHESTRATOR_AWS_S3_BUCKET_NAME", long, default_value = Some("madara-orchestrator-bucket"))]
pub bucket_name: Option<String>,
}| let run_cmd: RunCmd = RunCmd::parse(); | ||
|
|
||
| if run_cmd.setup { | ||
| setup_cloud(&run_cmd).await.expect("Failed to setup cloud"); |
There was a problem hiding this comment.
can build config and do that directly
| } | ||
|
|
||
| pub fn get_queue_name(&self, queue_type: QueueType) -> String { | ||
| format!("{}_{}_{}", self.sqs_prefix, queue_type, self.sqs_suffix) |
There was a problem hiding this comment.
can make this a queue value and not sqs
There was a problem hiding this comment.
Initially it was a part of AWSSQSValidatedArgs which was wrong, I have shifted it to be a part of SqsQueue for now, when we add another queue provider we can exercise this generalisation
| @@ -435,3 +455,121 @@ pub mod implement_client { | |||
| (rpc_url, starknet_client, server) | |||
| } | |||
| } | |||
|
|
|||
| struct EnvParams { | |||
There was a problem hiding this comment.
can it read from .env.test directly?
There was a problem hiding this comment.
Actually no, orchestrator binary has no way to know which .env to read from so it always reads from .env
here we read from .env.test and add it to .env for orchestrator to use
| impl Orchestrator { | ||
| pub fn setup(envs: Vec<(String, String)>) { | ||
| let port = get_free_port(); |
| let env_vec_2: Vec<(String, String)> = set_env_vars(); | ||
| env_vec.extend(env_vec_2); |
There was a problem hiding this comment.
can remove this if we remove env setup like above
| // "MADARA_ORCHESTRATOR_MADARA_BINARY_PATH".to_string(), | ||
| // get_env_var_or_panic("MADARA_ORCHESTRATOR_MADARA_BINARY_PATH"), | ||
| // ), | ||
| // Storage |
No description provided.