Skip to content
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

Draft: explore type state for FSRepository #924

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion crates/spfs-cli/cmd-clean/src/cmd_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use chrono::prelude::*;
use clap::Parser;
use colored::Colorize;
use miette::Result;
use spfs::storage::fs::NoRenderStoreForCurrentUser;
use spfs_cli_common as cli;
use spfs_cli_common::CommandName;

Expand Down Expand Up @@ -117,7 +118,14 @@ impl CommandName for CmdClean {

impl CmdClean {
pub async fn run(&mut self, config: &spfs::Config) -> Result<i32> {
let repo = spfs::config::open_repository_from_string(config, self.remote.as_ref()).await?;
// NoRenderStoreForCurrentUser is used to avoid attempting to create
// the directory if it missing, which may fail for full disks,
// preventing the clean operation from running.
let repo = spfs::config::open_repository_with_render_mode_from_string::<
_,
NoRenderStoreForCurrentUser,
>(config, self.remote.as_ref())
.await?;
tracing::debug!("spfs clean command called");

if let Some(runtime_name) = &self.remove_durable {
Expand Down
4 changes: 3 additions & 1 deletion crates/spfs-cli/main/src/cmd_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::PathBuf;

use clap::{Args, Subcommand};
use miette::Result;
use spfs::storage::fs::ValidRenderStoreForCurrentUser;

/// Create an empty filesystem repository
#[derive(Debug, Args)]
Expand Down Expand Up @@ -35,7 +36,8 @@ impl InitSubcommand {
pub async fn run(&self, _config: &spfs::Config) -> Result<i32> {
match self {
Self::Repo { path } => {
spfs::storage::fs::FsRepository::create(&path).await?;
spfs::storage::fs::FsRepository::<ValidRenderStoreForCurrentUser>::create(&path)
.await?;
Ok(0)
}
}
Expand Down
85 changes: 72 additions & 13 deletions crates/spfs/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
use storage::{FromConfig, FromUrl};
use tokio_stream::StreamExt;

use crate::storage::fs::ValidRenderStoreForCurrentUser;
use crate::{runtime, storage, tracking, Error, Result};

#[cfg(test)]
Expand Down Expand Up @@ -230,7 +231,9 @@ impl RemoteConfig {
pub async fn open(&self) -> storage::OpenRepositoryResult<storage::RepositoryHandle> {
let handle = match self.inner.clone() {
RepositoryConfig::Fs(config) => {
storage::fs::FsRepository::from_config(config).await?.into()
storage::fs::FsRepository::<ValidRenderStoreForCurrentUser>::from_config(config)
.await?
.into()
}
RepositoryConfig::Tar(config) => storage::tar::TarRepository::from_config(config)
.await?
Expand Down Expand Up @@ -376,7 +379,12 @@ impl Config {
}

/// Get the local repository instance as configured, creating it if needed.
pub async fn get_opened_local_repository(&self) -> Result<storage::fs::OpenFsRepository> {
pub async fn get_opened_local_repository<LocalRepositoryMode>(
&self,
) -> Result<storage::fs::OpenFsRepository<LocalRepositoryMode>>
where
LocalRepositoryMode: storage::fs::RenderStoreMode,
{
// Possibly use a different path for the local repository, depending
// on enabled features.
#[allow(unused_mut)]
Expand All @@ -388,7 +396,7 @@ impl Config {
Some(self.storage.root.join("ci").join(format!("pipeline_{id}")));
}

storage::fs::OpenFsRepository::create(
storage::fs::OpenFsRepository::<LocalRepositoryMode>::create(
use_ci_isolated_storage_path
.as_ref()
.unwrap_or(&self.storage.root),
Expand All @@ -400,12 +408,28 @@ impl Config {
})
}

/// Get the local repository instance as configured, creating it if needed.
///
/// The returned repo is guaranteed to be created, valid and open already. Ie
/// the local repository is not allowed to be lazily opened.
pub async fn get_local_repository_with_render_mode<LocalRepositoryMode>(
&self,
) -> Result<storage::fs::FsRepository<LocalRepositoryMode>>
where
LocalRepositoryMode: storage::fs::RenderStoreMode,
{
self.get_opened_local_repository::<LocalRepositoryMode>()
.await
.map(Into::into)
}

/// Get the local repository instance as configured, creating it if needed.
///
/// The returned repo is guaranteed to be created, valid and open already. Ie
/// the local repository is not allowed to be lazily opened.
pub async fn get_local_repository(&self) -> Result<storage::fs::FsRepository> {
self.get_opened_local_repository().await.map(Into::into)
self.get_local_repository_with_render_mode::<ValidRenderStoreForCurrentUser>()
.await
}

/// Get the local repository handle as configured, creating it if needed.
Expand All @@ -420,16 +444,20 @@ impl Config {
///
/// If `name` is defined, attempt to open the named remote
/// repository; otherwise open the local repository.
pub async fn get_remote_repository_or_local<S>(
pub async fn get_remote_repository_or_local<S, LocalRepositoryMode>(
&self,
name: Option<S>,
) -> Result<storage::RepositoryHandle>
where
S: AsRef<str>,
LocalRepositoryMode: storage::fs::RenderStoreMode,
{
match name {
Some(name) => self.get_remote(name).await,
None => Ok(self.get_local_repository().await?.into()),
None => Ok(self
.get_local_repository_with_render_mode::<LocalRepositoryMode>()
.await?
.into()),
}
}

Expand All @@ -441,10 +469,10 @@ impl Config {
}

/// Get a remote repository by name.
pub async fn get_remote<S: AsRef<str>>(
&self,
remote_name: S,
) -> Result<storage::RepositoryHandle> {
pub async fn get_remote<S>(&self, remote_name: S) -> Result<storage::RepositoryHandle>
where
S: AsRef<str>,
{
match self.remote.get(remote_name.as_ref()) {
Some(Remote::Address(remote)) => {
let config = RemoteConfig::from_address(remote.address.clone()).await?;
Expand Down Expand Up @@ -558,14 +586,20 @@ pub async fn open_repository<S: AsRef<str>>(
/// repository, use `config::get_remote_repository_or_local` instead.
///
/// When the repository specifier is a url, use `open_repository` instead.
pub async fn open_repository_from_string<S: AsRef<str>>(
pub async fn open_repository_with_render_mode_from_string<S, LocalRepositoryMode>(
config: &Config,
specifier: Option<S>,
) -> crate::Result<storage::RepositoryHandle> {
) -> crate::Result<storage::RepositoryHandle>
where
S: AsRef<str>,
LocalRepositoryMode: storage::fs::RenderStoreMode,
{
// Discovering that the given string is not a configured remote
// name is relatively cheap, so attempt to open a remote that
// way first.
let rh = config.get_remote_repository_or_local(specifier).await;
let rh = config
.get_remote_repository_or_local::<_, LocalRepositoryMode>(specifier)
.await;

if let Err(crate::Error::UnknownRemoteName(specifier)) = &rh {
// In the event that provided specifier was not a recognized name,
Expand Down Expand Up @@ -597,3 +631,28 @@ pub async fn open_repository_from_string<S: AsRef<str>>(
// No fallbacks worked so return the original result.
rh
}

/// Open a repository either by address or by configured name.
///
/// If `specifier` is `None`, return the configured local repository.
///
/// This function will try to interpret the given repository specifier
/// as either a url or configured remote name. It is recommended to use
/// an alternative function when the type of the specifier is known.
///
/// When the repository specifier is expected to be a configured
/// repository, use `config::get_remote_repository_or_local` instead.
///
/// When the repository specifier is a url, use `open_repository` instead.
pub async fn open_repository_from_string<S>(
config: &Config,
specifier: Option<S>,
) -> crate::Result<storage::RepositoryHandle>
where
S: AsRef<str>,
{
open_repository_with_render_mode_from_string::<_, ValidRenderStoreForCurrentUser>(
config, specifier,
)
.await
}
3 changes: 3 additions & 0 deletions crates/spfs/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@ where
};
match repo {
storage::RepositoryHandle::FS(r) => resolve_stack_to_layers_with_repo(stack, r).await,
storage::RepositoryHandle::FSNoUserRenders(r) => {
resolve_stack_to_layers_with_repo(stack, r).await
}
storage::RepositoryHandle::Tar(r) => resolve_stack_to_layers_with_repo(stack, r).await,
storage::RepositoryHandle::Rpc(r) => resolve_stack_to_layers_with_repo(stack, r).await,
storage::RepositoryHandle::FallbackProxy(r) => {
Expand Down
21 changes: 17 additions & 4 deletions crates/spfs/src/storage/fs/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ use futures::{Stream, StreamExt, TryFutureExt};
use graph::DatabaseView;
use tokio::io::{AsyncReadExt, AsyncWriteExt};

use super::RenderStoreMode;
use crate::graph::Object;
use crate::{encoding, graph, Error, Result};

#[async_trait::async_trait]
impl DatabaseView for super::FsRepository {
impl<T> DatabaseView for super::FsRepository<T>
where
T: RenderStoreMode,
{
async fn has_object(&self, digest: encoding::Digest) -> bool {
let Ok(opened) = self.opened().await else {
return false;
Expand Down Expand Up @@ -56,7 +60,10 @@ impl DatabaseView for super::FsRepository {
}

#[async_trait::async_trait]
impl graph::Database for super::FsRepository {
impl<T> graph::Database for super::FsRepository<T>
where
T: RenderStoreMode,
{
async fn write_object(&self, obj: &graph::Object) -> Result<()> {
self.opened().await?.write_object(obj).await
}
Expand All @@ -78,7 +85,10 @@ impl graph::Database for super::FsRepository {
}

#[async_trait::async_trait]
impl DatabaseView for super::OpenFsRepository {
impl<T> DatabaseView for super::OpenFsRepository<T>
where
T: RenderStoreMode,
{
async fn has_object(&self, digest: encoding::Digest) -> bool {
let filepath = self.objects.build_digest_path(&digest);
tokio::fs::symlink_metadata(filepath).await.is_ok()
Expand Down Expand Up @@ -124,7 +134,10 @@ impl DatabaseView for super::OpenFsRepository {
}

#[async_trait::async_trait]
impl graph::Database for super::OpenFsRepository {
impl<T> graph::Database for super::OpenFsRepository<T>
where
T: RenderStoreMode,
{
async fn write_object(&self, obj: &graph::Object) -> Result<()> {
let digest = obj.digest()?;
let filepath = self.objects.build_digest_path(&digest);
Expand Down
3 changes: 3 additions & 0 deletions crates/spfs/src/storage/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ pub use repository::{
read_last_migration_version,
Config,
FsRepository,
NoRenderStoreForCurrentUser,
OpenFsRepository,
Params,
RenderStore,
RenderStoreMode,
ValidRenderStoreForCurrentUser,
DURABLE_EDITS_DIR,
};
12 changes: 9 additions & 3 deletions crates/spfs/src/storage/fs/payloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ use std::pin::Pin;
use futures::future::ready;
use futures::{Stream, StreamExt, TryFutureExt};

use super::{FsRepository, OpenFsRepository};
use super::{FsRepository, OpenFsRepository, RenderStoreMode};
use crate::storage::prelude::*;
use crate::tracking::BlobRead;
use crate::{encoding, Error, Result};

#[async_trait::async_trait]
impl crate::storage::PayloadStorage for FsRepository {
impl<T> crate::storage::PayloadStorage for FsRepository<T>
where
T: RenderStoreMode,
{
async fn has_payload(&self, digest: encoding::Digest) -> bool {
let Ok(opened) = self.opened().await else {
return false;
Expand Down Expand Up @@ -52,7 +55,10 @@ impl crate::storage::PayloadStorage for FsRepository {
}

#[async_trait::async_trait]
impl crate::storage::PayloadStorage for OpenFsRepository {
impl<T> crate::storage::PayloadStorage for OpenFsRepository<T>
where
T: RenderStoreMode,
{
async fn has_payload(&self, digest: encoding::Digest) -> bool {
let path = self.payloads.build_digest_path(&digest);
tokio::fs::symlink_metadata(path).await.is_ok()
Expand Down
Loading