Saved Objects - make import/export stream based #39674
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💚 Build Succeeded |
| `); | ||
| }); | ||
|
|
||
| test('filters out empty lines', async () => { |
There was a problem hiding this comment.
This diff is hard to read. The filters out empty lines test was removed from here, but added here
legrego
added
Feature:Saved Objects
release_note:skip
This comment has been minimized.
This comment has been minimized.
💔 Build Failed |
| return h | ||
| .response(docsToExport.map(doc => stringify(doc)).join('\n')) | ||
| .response(docsToExport.join('\n')) |
There was a problem hiding this comment.
shouldn't we response with Stream? should reduce memory & CPU footprint as well
There was a problem hiding this comment.
I intentionally left this as a normal, non-streamed response. I didn't feel comfortable changing that as part of this PR. Maybe that's something we can investigate when we migrate these routes to the NP. Does the NP allow for streaming responses?
There was a problem hiding this comment.
| @@ -19,6 +19,12 @@ | |||
|
|
|||
| import { getSortedObjectsForExport } from './get_sorted_objects_for_export'; | |||
| import { SavedObjectsClientMock } from '../service/saved_objects_client.mock'; | |||
| import { Readable } from 'stream'; | |||
| import { createPromiseFromStreams, createConcatStream } from '../../../../legacy/utils/streams'; | |||
There was a problem hiding this comment.
a bit concern with importing utils from legacy, but we haven't discuss this inside the team.
There was a problem hiding this comment.
I left these utilities in legacy when I moved Saved Objects to Core, so it's a todo for the Saved Objects migration. There are stable NPM packages for many of these so I'm hoping we can replace the utilities with dedicated npm dependencies.
💚 Build Succeeded |
* transform ndjson within route handlers for SO import/export APIs * convert export saved objects to return a stream * fix stream creation
Summary
A prerequisite to #37286, this converts the import/export server APIs to be stream based.
Prior to this PR, the
importAPI accepted a stream containing an NDJSON file, and theexportAPI produced an array of saved objects. Now, theimportAPI accepts a stream of saved objects, and theexportAPI produces a stream of saved objects. The HTTP routes were updated accordingly to handle the streams.The motivation is to allow the result of the export operation to be directly used in the import operation server-side, similar to:
importSavedObjects(exportSavedObjects(...));.As a result, the transformation to/from NDJSON has been moved to the http route handlers, so that server-side consumers (such as #38014) do not need to concern themselves with this file format, but can instead use the streams directly.
Why streams instead of simple lists?
At one point, we had talked about updating the import/export APIs to accept lists of objects, but @mikecote mentioned that we were considering going stream based to eventually get relax the export/import size limit that exists today. The import API already accepted a stream, so it wasn't much of a lift to finish the conversion.
This does not change the public API
The public api remains unchanged by this PR. The request/response are not streamed as a result of this change.