process archive in mememory #75
Conversation
Signed-off-by: Pablo Chacin <[email protected]>
| // analizeMetadata extracts the dependencies from the metadata.json file | ||
| func analizeMetadata(content []byte) (analyzer, error) { | ||
| metadata := archiveMetadata{} | ||
| if err := json.Unmarshal(content, &metadata); err != nil { |
There was a problem hiding this comment.
If I'm not mistaken, we could use json.NewDecoder and then there is no need to pass bytes, but we could use io.Reader directly here
| continue | ||
| } | ||
| content := &bytes.Buffer{} | ||
| if _, err := io.CopyN(content, reader, maxFileSize); err != nil && !errors.Is(err, io.EOF) { |
There was a problem hiding this comment.
It's a corner case, but still, I'm thinking isn't that right for the files that are bigger than 10 MB 🤔 Shoudld we maybe write a warning log in that case.
If the other suggestion about analizeMetadata will be accepted, we could probably move these lines closer to the scriptAnalyzer
And perhaps not for that PR, but it's worth investigation if we can instead of copying content.Bytes() just pass around readers, in other words make possible to scriptAnalyzer also work with buffers
There was a problem hiding this comment.
It's a corner case, but still, I'm thinking isn't that right for the files that are bigger than 10 MB 🤔 Shoudld we maybe write a warning log in that case.
Not sure what you refer to in "isn't that right". Could you please elaborate?
There was a problem hiding this comment.
So my understanding that this line will copy at maximum 10MB, silently ignoring the rest, and I'm questioning if that's a right way to do. Yes, the risks that we processing bigger files where module usage located after 10MB of data is low, but still
There was a problem hiding this comment.
And perhaps not for that PR, but it's worth investigation if we can instead of copying content.Bytes() just pass around readers, in other words make possible to scriptAnalyzer also work with buffers
I definitely will do this. I started but realized it requires significant changes in other parts of the code and prefer to make this in a follow-up PR.
There was a problem hiding this comment.
Sure, like I said, both my comments are non-blocking, feel free to merge this as it is and continue in follow-up PRs 👍
Fixes multiple issues when trying to extract the archive and process it with k6pack, including issues with windows paths.
Also ensures that only the content of the archive is processed.
Closes #70