feat: allow passing a stream to KeyValueStore.setRecord

[gahabeen]

Right now the only way for a stream to be save in a KV store is using a buffer but (correct me if I'm wrong), that would mean using the memory (while passing the stream would directly send it via the network to the s3 bucket).

Current working solution:

async function concatStreamToBuffer(stream) {
    return new Promise((resolve, reject) => {
        const chunks = [];
        stream
            .on('data', (chunk) => {
                chunks.push(chunk);
            })
            .on('error', (e) => reject(e))
            .on('end', () => {
                const buffer = Buffer.concat(chunks);
                return resolve(buffer);
            });
    });
}

Apify.main(async () => {
    const store = await Apify.openKeyValueStore();

    const videoStream = new Stream();
    // ...
    const videoBuffer = await concatStreamToBuffer(videoStream);
    await store.setValue('video', videoBuffer, { contentType: 'video/mp4' });
});

The PR would let us do:

Apify.main(async () => {
    const store = await Apify.openKeyValueStore();
    const videoStream = new Stream();
    // ...
    await store.setValue('movie', videoStream, { contentType: 'video/mp4' });
});

Thoughts?

---

We uncovered this issue while working on a project with @levent91.

[B4nan]

What about some tests? :]

[mnmkng]

The underlying setRecord call already supports streams and I think it even validates them. I would skip the extra dependency and just relax the validation here in the SDK so that streams pass it. Or replicate the validation in the client, so that we don't get two different validations.

But a test would be nice anyway! 😄

[gahabeen]

Of course. Added tests. First wanted to discuss the matter :)

@mnmkng I feel the test is required as some restrictions are being applied to contentType for other value types (which I think is to allow saving objects as JSON without providing contentType).

Client lib implementation for reference: https://github.com/apify/apify-client-js/blob/master/src/resource_clients/key_value_store.ts

[gustavotr]

Nice timing, I am also waiting for this PR.

[gahabeen]

@B4nan This should be fixed. I like it better that way too 👍

[B4nan]

Not that it would matter much in here (given how small the PR is), but please dont rebase after you get a review, so we can see what you changed. We will squash merge the PR, so there will be just a single commit in master (and will have the commit message based on PR title).

[B4nan]

Weird, tests started to fail? Hard to say why as I dont see what you changed because of the rebase :P

[gahabeen]

I see that we have this tests that now fails as we've changed the behaviour.

And it's not the only one, the function one fails as well.
Returning Data after transformation must be a string, an ArrayBuffer, a Buffer, or a Stream which I presume comes back from outside the SDK.

[gahabeen]

Always beautiful when all lights turn 🟢 😎