-
Notifications
You must be signed in to change notification settings - Fork 255
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
Add environment variables provider #2670
Conversation
pub use statik::StaticVariables; | ||
|
||
use serde::de::DeserializeOwned; | ||
use spin_expressions::Provider; | ||
use spin_factors::anyhow; | ||
|
||
pub trait MakeVariablesProvider: 'static { |
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.
Do you think we should switch to the runtime resolver pattern 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.
That's the next PR 😄
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.
/// no .env file is loaded. | ||
pub fn new( | ||
prefix: Option<impl Into<String>>, | ||
env_fetcher: impl Fn(&str) -> Result<String, VarError> + Send + Sync + 'static, |
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.
Do we have a concrete need for this? Seems a bit marginal vs implementing an entirely new provider...
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.
It makes testing not rely on the environment it's run in. If it complicates too many things in your opinion, we can go back to just assuming std::env::variable
.
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 a separate with_env_fetcher
? 🤷
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
I don't know why all these other checks ran. Feel free to ignore them and merge. |
Adds a
EnvVariables
provider and moves the static provider to its own submodule. If this looks good, the next step is to implement the vault and azure extensions.