Skip to content
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

Add support for streaming result to a stream destination #4

Open
hawyar opened this issue May 26, 2021 · 5 comments
Open

Add support for streaming result to a stream destination #4

hawyar opened this issue May 26, 2021 · 5 comments
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@hawyar
Copy link
Contributor

hawyar commented May 26, 2021

Background

Currently we store the entire response body before returning it. While that works, certain datasets have very large response body. It would be inefficient to store it in memory, instead we can streams to a specified destination.

Proposed API Change(s)

Add support to stream the dataset response to a destination specified by the client. This can be done by adding streamTo as an option. The streamTo receives a stream destination.

Example saving the dataset content to a json file:

const CMSClient = createClient('5fr6-cch3', {
    output: 'json',
    includeMetadata: true,
    streamTo: require('fs').createWriteStream('./output.json');
  });

The example above demonstrates specifying a writable stream which then creates a file output.json filled with the response body

Breaking changes?

This change will require us to switch from using cross-fetch to node-fetch. Which means this package will no longer be isomorphic. Now do we only support node environment? Do we provide fallback if in a browser environment?

@hawyar hawyar added the enhancement New feature or request label May 26, 2021
@hawyar hawyar self-assigned this May 26, 2021
@ccali11
Copy link
Contributor

ccali11 commented May 28, 2021

@hawyar - This change makes sense to me. What would supporting both environments look like to you? Requiring both dependencies and then including in our documentation how to use client in browser vs. node? Or did you have something else in mind?

P.S. Once again, awesome documentation here! You're setting a great example for the team for effective communication. A+

@hawyar
Copy link
Contributor Author

hawyar commented May 28, 2021

The only difference between the APIs would be in node you can stream the response but in the browser you can't. We will provide documentation and the rest should remain the same. For the browser we can use esbuild to bundle cross-fetch with our package. I would love to hear other ways of handling this. Thank you @ccali11

@hawyar hawyar added the question Further information is requested label Jun 2, 2021
@shanejearley
Copy link

shanejearley commented Jun 16, 2021

Keeping this open so we can get back to work on resolving the esbuild implementation after #6 is merged.

@shanejearley
Copy link

@hawyar are we still trying to resolve the esbuild issue? Hoping to take another look if so!

@hawyar
Copy link
Contributor Author

hawyar commented Jun 23, 2021

@shanejearley Let's keep this open for now. I am down to take another look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants