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

db.add API docs are not correct; it can accept a stream as well as a string #263

Open
colleeseum opened this issue Jul 7, 2021 · 2 comments

Comments

@colleeseum
Copy link

colleeseum commented Jul 7, 2021

Loading file in memory to stream afterward is not efficient nor inline with Javascript mentality.

@jmrog
Copy link
Contributor

jmrog commented Jul 7, 2021

Thanks for this issue. However, I don't think stardog.js should accept a filename, because I don't think it should be in the business of reading files from a user's system (it's an HTTP wrapper library), and also because it is not generally possible at present to read a file in the browser given just a filename (things are starting to change a bit there, but just barely, and not in all major browsers).

More importantly, stardog.js already can accept a stream as an argument to db.add, so you don't actually need to load whole files into memory in order to send them. For example, in Node.js, you can do something like this (this is pseudo-code-ish, but expresses the idea):

const rdfStream = fs.createReadStream(filePath);
db.add(conn, dbId, transactionId, rdfStream, options).then(/* . . . */);

Probably the real issue here is that our API docs for db.add claim that the content argument must be a string, and that is false. We should update the docs. I'll change the name of this issue to reflect that.

@jmrog jmrog changed the title db.add should allow filename, rather than content db.add API docs are not correct; it can accept a stream as well as a string Jul 7, 2021
@colleeseum
Copy link
Author

It the typescript type must be adjusted as well. as a work around you can use

// @ts-ignore

to workaround this issue. Thanks @jmrog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants