-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Stdlib: user-friendly filesystem API #211
Comments
I'll take it. |
Out of curiosity what type of methods would you guys like for this lib to have I imagine
and maybe a few other directory ones are a must but anything else for a first iteration. |
In the brainstorming process for what this API might look like, I encountered a few contention points where design decisions should be made:
|
I would argue for using a new enum that could be more user friendly possibly, I also personally more prefer the |
|
Why would async be required for this? Maybe I'm not using the right term but I essentially mean a file handle |
Fair enough. I only think about streams in the context of async data processing (from node) but I suppose OCaml has blocking streams & channels 😛 I'd definitely like to stay away from that overloaded terminology if possible. |
I've made a pull request for this. Happy to have discussion about the api 😝 |
I'm working on an implementation in av8ta/fs based on feedback that looks something like in this comment: #1596 (comment) export enum Error<e> {
EISDIR(e),
ENOENT(e),
.... 75 more errors (just these two currently implemented to gather feedback)
UNKNOWN(e),
}
export record PathOpenOptions {
lookupFlag: List<File.LookupFlag>,
openFlags: List<File.OpenFlag>,
rights: List<File.Rights>,
inheriting: List<File.Rights>,
rwFlags: List<File.FdFlag>,
} I've implemented // -> Result<File.FileDescriptor, Error<String>>
export let open = (path: Path.Path, options: PathOpenOptions) => {} Recursively reads file in 1024 byte chunks. Opens & closes file descriptor. // -> Result<String, Error<String>>
export let readFile = (path: Path.Path) => {} Will begin working on export let readFileChunk = (handle: Fs.FileHandle, maxBytesToRead: Number) => {
// store handle if not seen previously
// read one chunk
// successive calls to readFileChunk advance
(chunk, numBytesRead)
}
export let readFileChunks = (handle: Fs.FileHandle, maxBytesToRead: Number, callback) => {
// store handle if not seen previously
// read each chunk & call back on each
// until end of file
callback((chunk, numBytesRead))
}
export let readFileSlice = (handle: Fs.FileHandle, range:Range.Range) => {
// store handle if not seen previously
// read chunk in range
(slice, numBytesRead)
}
export let readFileSlices = (handle: Fs.FileHandle, range:Range.Range, maxBytesToRead: Number, callback) => {
// store handle if not seen previously
// read chunk in range up to maxBytesToRead
// callback on each chunk
callback((chunk, numBytesRead))
} |
@av8ta Why are you having |
@phated Rather than load the whole file into memory and then work on it, devs can get work done on the data immediately. Think; logfile streaming in on stdin, processing the chunks, and then writing the transformation to stdout. Loads of other usecases of course because streams are a great way to work on data. Of course if they would rather do it in a loop then there are the singular versions I also think wasm on edge compute is one of the many great usecases for webassembly and grain is already great for cli tools. Those environments can be very resource constrained. I tried this and got a greatly improved output file :) let writeStdout = message => File.fdWrite(File.stdout, message)
let improveCsv = string => String.replaceAll(",", "🌾\n\n", string)
let callback = string => writeStdout(improveCsv(string))
readFileChunks(Fs.stdin, 1024, ((string, num)) => callback(string)) |
For this first iteration, let's keep the API really small and then we can expand it from there. We'll hold off on chunks/streaming for now, since that'll play a lot into Grain's async story, but we're a ways off from that. So for the first version, I propose this small API:
There are some natural extensions to this, like reading/writing a range, but those can come later. This gives us a really good starting point that we can continue to build from. This will need a long overdue update to sys/file to operate on bytes, but I'll get that change in swiftly. |
Here is said change: #1655 |
Looks good to me @ospencer, thanks for the feedback |
@av8ta Since you made a lot of progress towards this API on your initial PR, were you still planning on writing a new iteration based on the recent feedback or would you be ok with someone else bringing it over the finish line? |
@alex-snezhko I'm keen to resume with the new design. I found myself afk for a few weeks while shifting country and getting settled in, and can get back into it now. |
@av8ta Sounds good! I made some progress for the new proposed API that you could use as a base if you'd like https://github.com/alex-snezhko/grain/blob/ergo-fs/stdlib/sys/fs.gr |
Thanks @alex-snezhko that's really helpful |
Hey @av8ta and @alex-snezhko - I'd love for this PR to be up soon so we can get as much landed for 0.6 as possible. We are probably only a couple of weeks away! 🙏 |
Hey @phated I've had busy weekends lately, but I've got a chunk of this one free so I'll take a look at it. Hopefully that won't be too late for 0.6 🙏 |
Authored-by: Alex Snezhko <alexsnezhko89@gmail.com> Co-authored-by: av8ta <av8ta@pm.me>
@av8ta I've had some time to pick this back up as I wasn't sure if you had the time to get to this since your last comment. However, I see that you pushed up a commit on Oct 3rd, so I was curious if you were still looking to close this out or if you'd be fine with me making this PR? I implemented several additional functions and think I have it at a near-complete state now. |
@alex-snezhko yeah go for it, I had planned to but having trouble creating the time for it. |
No description provided.
The text was updated successfully, but these errors were encountered: