-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat: Enable .env file loading using env
feature
#1308
Changes from all commits
d378acd
401ef56
158a2f1
636c3dc
b68630e
de75607
eadcb55
d1c8fa3
fda375a
26540e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,22 @@ use std::{ | |
path::PathBuf, | ||
str::FromStr, | ||
}; | ||
use tracing::warn; | ||
use tracing::{ | ||
info, | ||
warn, | ||
}; | ||
use tracing_subscriber::{ | ||
filter::EnvFilter, | ||
layer::SubscriberExt, | ||
registry, | ||
Layer, | ||
}; | ||
|
||
#[cfg(feature = "env")] | ||
use dotenvy::dotenv; | ||
#[cfg(feature = "env")] | ||
use tracing::error; | ||
|
||
lazy_static::lazy_static! { | ||
pub static ref DEFAULT_DB_PATH: PathBuf = dirs::home_dir().unwrap().join(".fuel").join("db"); | ||
} | ||
|
@@ -41,6 +49,22 @@ pub enum Fuel { | |
pub const LOG_FILTER: &str = "RUST_LOG"; | ||
pub const HUMAN_LOGGING: &str = "HUMAN_LOGGING"; | ||
|
||
#[cfg(feature = "env")] | ||
fn init_environment() -> Option<PathBuf> { | ||
match dotenv() { | ||
Ok(path) => Some(path), | ||
Err(e) => { | ||
error!("Unable to load .env environment variables: {e}. Please check that you have created a .env file in your working directory."); | ||
None | ||
} | ||
} | ||
} | ||
|
||
#[cfg(not(feature = "env"))] | ||
fn init_environment() -> Option<PathBuf> { | ||
None | ||
} | ||
|
||
pub async fn init_logging() -> anyhow::Result<()> { | ||
let filter = match env::var_os(LOG_FILTER) { | ||
Some(_) => { | ||
|
@@ -87,6 +111,11 @@ pub async fn init_logging() -> anyhow::Result<()> { | |
} | ||
|
||
pub async fn run_cli() -> anyhow::Result<()> { | ||
init_logging().await?; | ||
if let Some(path) = init_environment() { | ||
let path = path.display(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to add a test to verify that it works properly with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm adding tests to ensure the basic behaviour of the CLI:
I am re-reading your comment here, but I'm unsure if I understood what to use snapshot for here. There should never be a If the goal of a snapshot is to help developers, one thing we can do is add an |
||
info!("Loading environment variables from {path}"); | ||
} | ||
let opt = Opt::try_parse(); | ||
if opt.is_err() { | ||
let command = run::Command::try_parse(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it also should be part of the
production
anddebug
featureThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can understand adding it to the
debug
feature. I can definitely add it there.I was looking at some of the deployment architecture to see if there would be any conflicts here, because I don't know how this will interact with existing environment variables like the ones set in K8s any configs or helm. It may override them. Is there any risk here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't use
.env
, it shouldn't affect the current operator or chart deployment