-
Notifications
You must be signed in to change notification settings - Fork 18
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]: small refactoring of the Spin engine #63
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Radu Matei <[email protected]>
Signed-off-by: Radu Matei <[email protected]>
l.config.media_type().to_string() == spin_oci::client::SPIN_APPLICATION_MEDIA_TYPE | ||
}) | ||
.context("cannot find locked application in layers for application")?; | ||
let mut locked_app = LockedApp::from_json(&locked_layer.layer)?; |
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.
TODO: this should handle archive layers (ref #28)
@@ -0,0 +1,133 @@ | |||
use std::path::{Path, PathBuf}; |
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.
TODO: tests for the new loader.
containerd-shim-spin/src/engine.rs
Outdated
} | ||
} | ||
|
||
#[cfg(test)] |
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.
TODO: re-add these tests.
containerd-shim-spin/src/main.rs
Outdated
|
||
fn main() { | ||
// Configure the shim to have only error level logging for performance improvements. | ||
let shim_config = Config { | ||
default_log_level: "error".to_string(), | ||
// default_log_level: "error".to_string(), |
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.
TODO:
containerd-shim-spin/src/engine.rs
Outdated
use trigger_command::CommandTrigger; | ||
use trigger_sqs::SqsTrigger; | ||
// use trigger_command::CommandTrigger; | ||
// use trigger_sqs::SqsTrigger; |
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.
TODO: re-enable the command and SQS triggers
Signed-off-by: Radu Matei <[email protected]>
I think many of the changes in this PR are good. Do you intend to continue working on this PR, @radu-matei ? |
Yes — I would still like to bring these changes in. |
This is a WIP of beginning to refactor the Spin engine.
So far, most of this is preference, but this branch will try to address some of the TODOs in the code base eventually.
cc @Mossaka