IBM / core-dump-handler

Save core dumps from a Kubernetes Service or RedHat OpenShift to an S3 protocol compatible object store
https://ibm.github.io/core-dump-handler/
MIT License
136 stars 40 forks source link

fix loading .env by composer #91

Closed aleksey-novikov closed 2 years ago

aleksey-novikov commented 2 years ago

This fix early load .env file to override some variables like FILENAME_TEMPLATE

No9 commented 2 years ago

Thanks for this - Good catch Rather than loading the .env file twice we should probably just change the order of the call in https://github.com/IBM/core-dump-handler/blob/9ce51820f4f65bd5212b7ede2b64560c4282066a/core-dump-composer/src/main.rs#L19

So

    let mut cc = config::CoreConfig::new()?;
    cc.set_namespace("default".to_string());
    let mut envloadmsg = String::from("Loading .env");
    let l_dot_env_path = cc.dot_env_path.clone();
    match dotenv::from_path(l_dot_env_path) {
        Ok(v) => v,
        Err(e) => envloadmsg = format!("no .env file found so using Debug level logging {}", e),
    }

Should be

    let mut envloadmsg = String::from("Loading .env");
    let l_dot_env_path = cc.dot_env_path.clone();
    match dotenv::from_path(l_dot_env_path) {
        Ok(v) => v,
        Err(e) => envloadmsg = format!("no .env file found so using Debug level logging {}", e),
    }
    let mut cc = config::CoreConfig::new()?;
    cc.set_namespace("default".to_string());

Let me know what you think

No9 commented 2 years ago

@aleksey-novikov I'm going to bundle up some changes this weekend Do you want to apply the suggested change or are you ok for me to land it an close this?

No9 commented 2 years ago

OK I am assuming you've rightly checked out for the weekend - I am going to merge this so the contribution is recognized then apply refactor afterwards.