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.

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

Conversation

radu-matei
Copy link
Member

ref #455

Before this commit, for a spin.toml file that had a URL module source for a component (such as the file server and redirect templates), subsequent runs of spin up would always download the module from the URL before starting the application.

#1014 added a content-addressed registry cache that can be used to store remote module sources.

This commit updates the Spin local loader to use that cache to store the component sources referenced through URL in spin.toml.

Before fetching a component, the loader will check the local cache first. If present, it will read the file and return its content; otherwise, it will fetch it, store it in the cache, then return its content.

Currently, the default cache root is always used. Once spin up will have a way to configure it, that will be propagated here as well.

Signed-off-by: Radu Matei [email protected]

Before this commit, for a `spin.toml` file that had a URL module source
for a component (such as the file server and redirect templates),
subsequent runs of `spin up` would always download the module from the
URL before starting the application.

spinframework#1014 added a content-addressed
registry cache that can be used to store remote module sources.

This commit updates the Spin local loader to use that cache to store
the component sources referenced through URL in `spin.toml`.

Before fetching a component, the loader will check the local cache
first. If present, it will read the file and return its content;
otherwise, it will fetch it, store it in the cache, then return its
content.

Currently, the default cache root is always used. Once `spin up` will
have a way to configure it, that will be propagated here as well.

Signed-off-by: Radu Matei <[email protected]>
Copy link
Contributor

@vdice vdice left a comment

Choose a reason for hiding this comment

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

This is working great for me!

.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

@michelleN
Copy link
Collaborator

We should add an e2e test for this scenario so we can capture the behavior somewhere and make sure it continues working as expected.

@@ -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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants