Skip to content

Add initial cache for Spin URL component sources #1088

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Add initial cache for Spin URL component sources #1088

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 39 additions & 22 deletions crates/loader/src/local/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use spin_manifest::{
};
use tokio::{fs::File, io::AsyncReadExt};

use crate::{bindle::BindleConnectionInfo, digest::bytes_sha256_string};
use crate::{bindle::BindleConnectionInfo, digest::bytes_sha256_string, oci::cache::Cache};
Copy link
Collaborator

@lann lann Feb 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache should be refactored out of the oci mod (up to spin_loader::cache, presumably).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption is that when we do end up refactoring the loader crate and cleanup any unused Bindle code, this refactor would happen.

use config::{
FileComponentUrlSource, RawAppInformation, RawAppManifest, RawAppManifestAnyVersion,
RawAppManifestAnyVersionPartial, RawComponentManifest, RawComponentManifestPartial,
Expand Down Expand Up @@ -297,29 +297,46 @@ impl UrlSource {

/// Gets the data from the source as a byte buffer.
pub async fn get(&self) -> anyhow::Result<Vec<u8>> {
let response = reqwest::get(self.url.clone())
.await
.with_context(|| format!("Error fetching source URL {}", self.url))?;
// TODO: handle redirects
let status = response.status();
if status != reqwest::StatusCode::OK {
let reason = status.canonical_reason().unwrap_or("(no reason provided)");
anyhow::bail!(
"Error fetching source URL {}: {} {}",
self.url,
status.as_u16(),
reason
);
}
let body = response
.bytes()
.await
.with_context(|| format!("Error loading source URL {}", self.url))?;
let bytes = body.into_iter().collect_vec();
// TODO: when `spin up` integrates running an app from OCI, pass the configured
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add an issue to track this after this PR is merged?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fibonacci1729 I for one had forgotten about this comment when you were doing spin up --from-registry - did you already deal to it, or should we resurrect it as an enhancement request

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itowlson I completely missed this. We should about an issue. If you haven't got to it by your today, I'll open an issue during my tomorrow

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got in there quick so I could assign it to you. flees #1188

// cache root to this function. For now, use the default cache directory.
let cache = Cache::new(None).await?;
match cache.wasm_file(self.digest_str()) {
Ok(p) => {
tracing::debug!(
"Using local cache for module source {} with digest {}",
&self.url,
&self.digest_str()
);
Ok(tokio::fs::read(p).await?)
}
Err(_) => {
tracing::debug!("Pulling module from URL {}", &self.url);
let response = reqwest::get(self.url.clone())
.await
.with_context(|| format!("Error fetching source URL {}", self.url))?;
// TODO: handle redirects
let status = response.status();
if status != reqwest::StatusCode::OK {
let reason = status.canonical_reason().unwrap_or("(no reason provided)");
anyhow::bail!(
"Error fetching source URL {}: {} {}",
self.url,
status.as_u16(),
reason
);
}
let body = response
.bytes()
.await
.with_context(|| format!("Error loading source URL {}", self.url))?;
let bytes = body.into_iter().collect_vec();

self.digest.verify(&bytes).context("Incorrect digest")?;
self.digest.verify(&bytes).context("Incorrect digest")?;
cache.write_wasm(&bytes, self.digest_str()).await?;

Ok(bytes)
Ok(bytes)
}
}
}
}

Expand Down