refactor: create image trait with an impl for docker and podman

[tylerslaton]

Another PR in a series of refactors that I'm doing that are aimed at reducing the complexity and extensibility of our code base. This one creates a new Image trait with two implementations for docker and podman so they can have their own unique functionality.

[exdx]

Very nice! I had some feedback, largely around naming, but this is definitely a step in the right direction to clean up the codebase!

[exdx]

nit: I think this should go under the stop() function for clarity

[tylerslaton]

Which part, creating the error?

[exdx]

No, I meant stopping the container. Stopping and deleting a container are related, so they can both go under stop()

[tylerslaton]

Ah so are you saying that instead of copy_files() starting and stopping the container via start() and stop() we should have those functions be called outside of copy_files() directly in the lib.rs run function? Something like this:

// src/lib.rs - fn run()

// Create the container
let container_id = match container.start().await {
    Ok(id) => id,
    Err(e) => {
        return Err(anyhow!("failed to start the image: {}", e));
}

// Copy files from the image
match container
    .copy_files(
        config.content_path,
        config.download_path,
        config.write_to_stdout,
        container_id,
    )
    .await
{
    Ok(_) => {}
    Err(e) => {
        return Err(anyhow!("❌ error copying the image's files: {}", e));
    }
}

// Stop the container
match container.stop(container_id).await {
    Ok(_) => {}
    Err(e) => {
        return Err(anyhow!("failed to stop the image: {}", e));
    }
}

[tylerslaton]

@exdx Okay with the latest push I broke out the runtime oriented logic into its own module folder along with the changes you were asking for in the rebase. This should make it easier to modularize that logic moving forward. What are your thoughts?

[exdx]

LGTM! Great job working with async traits. Feel free to merge.

The only remaining nit I had was to move the container stop logic into the stop() function instead of in the copy_files() function, since it's more related to stopping and removing the container

[exdx]

Is #57 addressed by these changes, or do we still need to look into it?

[tylerslaton]

Is #57 addressed by these changes, or do we still need to look into it?

The underlying logic of present_locally didn't change here so I'll need to investigate before giving you a concrete answer here.