Introduces Reader into k6deps.Source #69
Conversation
|
cc also: @pablochacin |
| @@ -70,7 +99,7 @@ func (opts *Options) lookupEnv(key string) (string, bool) { | |||
| } | |||
|
|
|||
| func loadSources(opts *Options) error { | |||
| if !opts.Archive.Ignore && (len(opts.Archive.Contents) > 0 || len(opts.Archive.Name) > 0) { | |||
| if !opts.Archive.Ignore && !opts.Archive.IsEmpty() { | |||
There was a problem hiding this comment.
Also, maybe a side-question for Ivan/Pablo, why the loadArchive is uniq the way we load, I mean it seems like if archive provided we completely ignore other paths, like manifest/env, could it cause any side effects? 🤔
|
@olegbespalov I see the value of trying to move some work in-memory, but I fail to see how this PR accomplished this objective. Introducing the reader just skips the step of opening the archive, but it is still extracted into a temporary directory, where it's processed with I see the objective you propose could be achieved by using a file system abstraction to process the archive, allowing for an in-memory processing. There are even packages that implement the fs interface on top of an |
This PR doesn't aim to fix everything, like you pointed out (and definitely I saw it in the code) we have a dependency of the Where it still brings value, as the first step, is the way how we can reduce the steps when we use the analysis. Imagine the use case where you have an archive stored remotely. So right now you have two options:
The first option introduces additional overhead of managing the files, the second has the downside that you need to allocate the big chink of memory (imagine big archive) when passing the content. But working with reader, we can just pipe the response body and process it efficiently, or fully efficiently once everything will be adjusted. |
What?
While one of the internal reviews of another pull request that relies on that library, there was a valid concern raised that if we provide a go library it makes more sense to rely on the standard interfaces and use less file system, which is even currently seems to be possible, but it requires full copy of archive.
This minor change makes it possible to use directly
io.Readerin thek6deps.SourceWhy?
Make API more go idiomatic and potentially reduce some steps