Switch NetworksService, VolumesService, EntityStore to FilePath.#1493
Switch NetworksService, VolumesService, EntityStore to FilePath.#1493jglogan merged 2 commits intoapple:mainfrom
Conversation
| private static func withTempDir<T: Sendable>( | ||
| _ body: @Sendable (FilePath) async throws -> T | ||
| ) async throws -> T { | ||
| let url = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) | ||
| try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) | ||
| defer { try? FileManager.default.removeItem(at: url) } | ||
| return try await body(FilePath(url.path)) | ||
| } |
There was a problem hiding this comment.
Nice, might be a good utility to pull out into a public test utils
|
|
||
| let resourceRoot = appRoot.appendingPathComponent("volumes") | ||
| // TODO: This goes away when we convert our roots to FilePath | ||
| let appPath = FilePath(appRoot.absolutePath()) |
There was a problem hiding this comment.
Where does .absolutePath() come from? Can't seem to find it on the swift documentation https://developer.apple.com/documentation/foundation/url
There was a problem hiding this comment.
It actually comes from ContainerizationOCI URL+Extensions.swift.
| let resourceRoot = appRoot.appendingPathComponent("volumes") | ||
| // TODO: This goes away when we convert our roots to FilePath | ||
| let appPath = FilePath(appRoot.absolutePath()) | ||
| let resourceRoot = appPath.appending("networks") |
There was a problem hiding this comment.
Why is this changing from "volumes" to "networks"
There was a problem hiding this comment.
Has to be a copy/paste error. Apparently our tests are okay with volumes and networks being placed in the same directory...
| try FileManager.default.createDirectory(atPath: entityPath.string, withIntermediateDirectories: true) | ||
| let data = try encoder.encode(entity) | ||
| try data.write(to: metadataUrl) | ||
| try data.write(to: URL(filePath: metadataPath.string)) |
There was a problem hiding this comment.
For later: If we convert everything to FilePath it might be nice to introduce some extensions to FilePath such as func asURL() to make the api a bit nicer.
| public func upsert(_ entity: T) async throws { | ||
| let metadataUrl: URL = metadataUrl(entity.id) | ||
| let entityPath = try entityPath(entity.id) | ||
| try FileManager.default.createDirectory(atPath: entityPath.string, withIntermediateDirectories: true) |
There was a problem hiding this comment.
Why do we need to create the directory now?
There was a problem hiding this comment.
I would say it's because no code calls it today, and previously no unit tests existed.
3 participants
Type of Change
Motivation and Context
URL for filesystem paths is tech debt.
Testing