-
Notifications
You must be signed in to change notification settings - Fork 52
Json Progress File #391
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
Json Progress File #391
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import Foundation | ||
import SwiftlyCore | ||
import SystemPackage | ||
import TSCUtility | ||
|
||
enum ProgressInfo: Codable { | ||
case step(timestamp: Date, percent: Int, text: String) | ||
case complete(success: Bool) | ||
} | ||
|
||
struct JsonFileProgressReporter: ProgressAnimationProtocol { | ||
let filePath: FilePath | ||
private let encoder: JSONEncoder | ||
private let ctx: SwiftlyCoreContext | ||
private let fileHandle: FileHandle | ||
|
||
init(_ ctx: SwiftlyCoreContext, filePath: FilePath, encoder: JSONEncoder = JSONEncoder()) throws | ||
{ | ||
self.ctx = ctx | ||
self.filePath = filePath | ||
self.encoder = encoder | ||
self.fileHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: filePath.string)) | ||
} | ||
|
||
private func writeProgress(_ progress: ProgressInfo) { | ||
let jsonData = try? self.encoder.encode(progress) | ||
guard let jsonData = jsonData else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thought: The progress animation protocol is limiting here because nothing is async. This message, while probably not very likely to happen, might not get presented in time because it's emitted in a separate task. If we did want to revisit this, I expect that we would make our own protocol that's async friendly, and some kind of a shim for the PercentProgressAnimation. |
||
Task { [ctx = self.ctx] in | ||
await ctx.message("Failed to encode progress entry to JSON") | ||
} | ||
return | ||
} | ||
|
||
self.fileHandle.write(jsonData) | ||
self.fileHandle.write("\n".data(using: .utf8) ?? Data()) | ||
try? self.fileHandle.synchronize() | ||
} | ||
|
||
func update(step: Int, total: Int, text: String) { | ||
guard total > 0 && step <= total else { | ||
return | ||
} | ||
self.writeProgress( | ||
ProgressInfo.step( | ||
timestamp: Date(), | ||
percent: Int(Double(step) / Double(total) * 100), | ||
text: text | ||
)) | ||
} | ||
|
||
func complete(success: Bool) { | ||
self.writeProgress(ProgressInfo.complete(success: success)) | ||
} | ||
|
||
func clear() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Where's the requirement for a progress file clear action coming from? I feel that this isn't a necessary function, and is likely to fail on a pipe instead of a regular file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is part of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think that a no-op makes sense here. The next progress entry will probably go backwards, and it will be up to a client to handle that case. |
||
// not implemented for JSON file reporter | ||
} | ||
|
||
func close() throws { | ||
try self.fileHandle.close() | ||
} | ||
} |
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.
suggestion: Let's spell out that the contract that the client must create this file first, and swiftly will append to it as it progresses.
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.
Added:"The file must be writable, else an error will be thrown"