feat: Moving bundle creation from ContainerService to SandboxService #1076
Conversation
|
Needs a |
Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift
Show resolved
Hide resolved
|
@ajemory Could you rebase this onto main and push the PR again? As it stands hawkeye is freaking out on the header checks. |
Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift
Outdated
Show resolved
Hide resolved
Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift
Show resolved
Hide resolved
| // Create bundle if it doesn't exist | ||
| if !(await self.bundleExists(at: path)) { | ||
| let metadataPath = Self.getMetadataPath(for: id) | ||
| try await sandboxClient.createBundle(metadataPath: metadataPath) | ||
| } |
There was a problem hiding this comment.
Could we just have the SandboxService call bundle.create in bootstrap? We don't really need the bundle created in here right?
There was a problem hiding this comment.
Just to clarify, you're suggesting to call bundle.create from SandboxService bootstrap instead of ContainersService bootstrap? Yeah, I think that's a better solution. I'll make the changes!
| let metadataPath = FileManager.default.temporaryDirectory | ||
| .appendingPathComponent("bundle-metadata-\(configuration.id).json") | ||
|
|
There was a problem hiding this comment.
Can we use getMetadataPath here instead?
|
|
||
| /// Get metadata file path for the given container ID | ||
| private static func getMetadataPath(for containerID: String) -> URL { | ||
| FileManager.default.temporaryDirectory |
There was a problem hiding this comment.
I think I'd prefer we just write the metadata to the bundle directory itself instead of a temporary location but also welcome to other thoughts on this. I think that way we just delete the bundle when the container is cleaned up and we're good to go. Plus then we know the file path in the SandboxService for free.
5 participants
Type of Change
Motivation and Context
Closes #1046 -- Right now we're creating container bundles in ContainersService. Move this to the SandboxService to make it easier to support different container bundle types.
Testing