-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-2116: [JS] implement IPC writers #2035
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
Changes from all commits
dc92b83
ef1acc7
6522cb0
43c671f
c8ba1fe
ae1f481
efb840f
18b9dd2
aaec76b
d98e178
7fff99e
4333e54
85eb7ee
a9d773d
508f4f8
da0f457
a242da8
db02c1c
402187e
304e75d
73a2fa9
4e80851
e75da13
78cba38
263d06d
832cc30
de81ac1
af9f4a8
3187732
b52af25
d4b8637
0be6de3
c0b88c2
ccaf489
081fefc
a79334d
a6a7ab9
2df1a4a
ed85572
a06180b
efc7225
7924e67
df43bc5
14e6b38
f497f7a
4ed6554
b765b12
e34afaa
1a9864c
4594fe3
7a346dc
917c2fc
261a864
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| #! /usr/bin/env node | ||
|
|
||
| // Licensed to the Apache Software Foundation (ASF) under one | ||
| // or more contributor license agreements. See the NOTICE file | ||
| // distributed with this work for additional information | ||
| // regarding copyright ownership. The ASF licenses this file | ||
| // to you under the Apache License, Version 2.0 (the | ||
| // "License"); you may not use this file except in compliance | ||
| // with the License. You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, | ||
| // software distributed under the License is distributed on an | ||
| // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| // KIND, either express or implied. See the License for the | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| const encoding = 'binary'; | ||
| const ext = process.env.ARROW_JS_DEBUG === 'src' ? '.ts' : ''; | ||
| const { util: { PipeIterator } } = require(`../index${ext}`); | ||
| const { Table, serializeStream, fromReadableStream } = require(`../index${ext}`); | ||
|
|
||
| (async () => { | ||
| // Todo (ptaylor): implement `serializeStreamAsync` that accepts an | ||
| // AsyncIterable<Buffer>, rather than aggregating into a Table first | ||
| const in_ = process.argv.length < 3 | ||
| ? process.stdin : fs.createReadStream(path.resolve(process.argv[2])); | ||
| const out = process.argv.length < 4 | ||
| ? process.stdout : fs.createWriteStream(path.resolve(process.argv[3])); | ||
| new PipeIterator(serializeStream(await Table.fromAsync(fromReadableStream(in_))), encoding).pipe(out); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to wrap this logic into a method on public serializeTo (out) {
new PipeIterator(serializeStream(this, 'binary')).pipe(out)
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TheNeuralBit yeah, that's a bit what the comment above is about. Ideally we don't have to aggregate the input into a table, instead just stream the writeStreamAsync: (input: AsyncIterable<Schema | RecordBatch>) => AsyncIterable<Uint8Array>;That said, the table should also have a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah that's all I was getting at |
||
|
|
||
| })().catch((e) => { console.error(e); process.exit(1); }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this fixed now in upstream Flatbuffers?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wesm I was able to update the version of
google-closure-compilerwe use in commit 2df1a4a, which allowed me to remove a number of hacks we had in the build to create the ES5/UMD bundle. One of those hacks was that we had to generate two versions of the FlatBuffers code, once in legacy ES5 JS that closure could use, and the TypeScript used everything else. With the new update, closure-compiler doesn't choke on the compiled TS output anymore.