Skip to content

Commit

Permalink
Merge pull request #2791 from fermyon/fix-kv-default-path
Browse files Browse the repository at this point in the history
Fix the path for default key-value stores
  • Loading branch information
lann authored Sep 3, 2024
2 parents 74d3517 + 8129386 commit 8fda052
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 19 deletions.
21 changes: 9 additions & 12 deletions crates/factor-key-value-spin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
path::{Path, PathBuf},
};

use anyhow::{bail, Context};
use anyhow::Context as _;
use serde::{Deserialize, Serialize};
use spin_factor_key_value::runtime_config::spin::MakeKeyValueStore;
use spin_key_value_sqlite::{DatabaseLocation, KeyValueSqlite};
Expand Down Expand Up @@ -42,23 +42,20 @@ impl MakeKeyValueStore for SpinKeyValueStore {
let path = resolve_relative_path(path, base_path);
DatabaseLocation::Path(path)
}
// If the base path is `None` but path is an absolute path, use the absolute path
(None, Some(path)) if path.is_absolute() => DatabaseLocation::Path(path.clone()),
// If the base path is `None` but path is a relative path, error out
(None, Some(path)) => {
bail!(
"key-value store path '{}' is relative, but no base path is set",
path.display()
)
}
// If the base path is `None` but use the path without resolving relative to the base path.
(None, Some(path)) => DatabaseLocation::Path(path.clone()),
// Otherwise, use an in-memory database
(None | Some(_), None) => DatabaseLocation::InMemory,
};
if let DatabaseLocation::Path(path) = &location {
// Create the store's parent directory if necessary
if let Some(parent) = path.parent().filter(|p| !p.exists()) {
fs::create_dir_all(parent)
.context("Failed to create key value store's parent directory")?;
fs::create_dir_all(parent).with_context(|| {
format!(
"failed to create key value store's parent directory: '{}",
parent.display()
)
})?;
}
}
Ok(KeyValueSqlite::new(location))
Expand Down
2 changes: 1 addition & 1 deletion crates/key-value-sqlite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ rusqlite = { version = "0.29.0", features = ["bundled"] }
spin-core = { path = "../core" }
spin-key-value = { path = "../key-value" }
spin-world = { path = "../world" }
tokio = "1"
tokio = { version = "1", features = ["rt-multi-thread"] }
tracing = { workspace = true }

[lints]
Expand Down
5 changes: 5 additions & 0 deletions crates/key-value-sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ pub struct KeyValueSqlite {
}

impl KeyValueSqlite {
/// Create a new `KeyValueSqlite` store manager.
///
/// If location is `DatabaseLocation::InMemory`, the database will be created in memory.
/// If it's `DatabaseLocation::Path`, the database will be created at the specified path.
/// Relative paths will be resolved against the current working directory.
pub fn new(location: DatabaseLocation) -> Self {
Self {
location,
Expand Down
3 changes: 3 additions & 0 deletions crates/key-value/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ impl StoreManager for DelegatingStoreManager {
}

fn summary(&self, store_name: &str) -> Option<String> {
if let Some(store) = self.delegates.get(store_name) {
return store.summary(store_name);
}
(self.default_manager)(store_name)?.summary(store_name)
}
}
Expand Down
17 changes: 11 additions & 6 deletions crates/runtime-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,14 @@ where
provided_log_dir: UserProvidedPath,
use_gpu: bool,
) -> anyhow::Result<Self> {
let runtime_config_dir = runtime_config_path
.and_then(Path::parent)
.map(ToOwned::to_owned);
let toml_resolver =
TomlResolver::new(&toml, local_app_dir, provided_state_dir, provided_log_dir);
let tls_resolver = runtime_config_path.map(SpinTlsRuntimeConfig::new);
let key_value_config_resolver = key_value_config_resolver(toml_resolver.state_dir()?);
let tls_resolver = runtime_config_dir.clone().map(SpinTlsRuntimeConfig::new);
let key_value_config_resolver =
key_value_config_resolver(runtime_config_dir, toml_resolver.state_dir()?);
let sqlite_config_resolver = sqlite_config_resolver(toml_resolver.state_dir()?)
.context("failed to resolve sqlite runtime config")?;

Expand Down Expand Up @@ -396,9 +400,11 @@ const DEFAULT_KEY_VALUE_STORE_LABEL: &str = "default";
/// The key-value runtime configuration resolver.
///
/// Takes a base path that all local key-value stores which are configured with
/// relative paths will be relative to.
/// relative paths will be relative to. It also takes a default store base path
/// which will be used as the directory for the default store.
pub fn key_value_config_resolver(
local_store_base_path: Option<PathBuf>,
default_store_base_path: Option<PathBuf>,
) -> key_value::RuntimeConfigResolver {
let mut key_value = key_value::RuntimeConfigResolver::new();

Expand All @@ -417,13 +423,12 @@ pub fn key_value_config_resolver(
.unwrap();

// Add handling of "default" store.
let default_store_path = default_store_base_path.map(|p| p.join(DEFAULT_SPIN_STORE_FILENAME));
// Unwraps are safe because the store is known to be serializable as toml.
key_value
.add_default_store::<SpinKeyValueStore>(
DEFAULT_KEY_VALUE_STORE_LABEL,
SpinKeyValueRuntimeConfig::new(
local_store_base_path.map(|p| p.join(DEFAULT_SPIN_STORE_FILENAME)),
),
SpinKeyValueRuntimeConfig::new(default_store_path),
)
.unwrap();

Expand Down

0 comments on commit 8fda052

Please sign in to comment.