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

What is required to be able to handle streams in _dbop()? #13

Closed
jesseclark opened this issue Mar 13, 2014 · 4 comments
Closed

What is required to be able to handle streams in _dbop()? #13

jesseclark opened this issue Mar 13, 2014 · 4 comments

Comments

@jesseclark
Copy link
Contributor

Hello,

First off thanks for creating these node bindings for BaseX. I think it is going to be a big help on my current project.

I'm building a web api that receives xml files in a post request and adds them to my BaseX db. Currently I am writing the received file to disk then opening it and passing it as string data to the Session.add() method.

It would be nice if I could stream the data directly to add() without having to hit the disk. I have access to the incoming readStream but it looks like stream support has not been added in _dbop().

Did you have a solution for this in mind? If you could give me an overview perhaps I could help implement it.

It looks like the action hinges around line 320 of index.js in sendQueueItem():

self.send(cmd.send);

and in this.send() line 144:

if (typeof s === "function")
            s = s();

so it seems like we would need to modify _dbop() to check if the input is a string or a stream and if it is a stream, instead of passing the command as a string like so (line 219 this._dbop()):

send : op + path + "\0" + input+"\0",

add a reference to the stream and other command components to the command object and make send a reference to a function that will read the stream:

send : this.readStream,
op: op,
path: path,
input: input 

and then this.readStream() could read the input stream and then return the assembled command in the format:

return op + path + "\0" + streamData + "\0";

I'm still a relative beginner with node so I might be a bit off on my proposed solution and would appreciate any feedback or let me know what you were thinking for a solution.

Best,
-Jesse

@apb2006
Copy link
Member

apb2006 commented Mar 13, 2014

Hi,
Thanks for your comments.
I did put some thought into streaming and I think it will require quite a few changes, particularly to support streaming from, as well as to, BaseX.
I would like to take the oportunity to fix some early design decisions I regret e.g including info in the result. This will create some incompatabities. I hope to create a branch for this soon.

@jesseclark
Copy link
Contributor Author

As time allows, I might take a crack at hacking in a solution to get streaming TO BaseX working as that would be a big help for my current use case. I can finish my project without it but it would be a bit of a performance boost to stream directly to the db without saving an intermediate file.

@apb2006
Copy link
Member

apb2006 commented Apr 10, 2014

The branch stream contains a streaming TO BaseX implementation. Sample use here. I plan to merge this soon, if you have time to check it looks ok for your needs that would be great.

@apb2006
Copy link
Member

apb2006 commented Sep 29, 2021

merged some time ago

@apb2006 apb2006 closed this as completed Sep 29, 2021
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