search

evanplaice / node-ftpsync

Intelligent file syncronization over FTP
http://evanplaice.github.io/node-ftpsync
MIT License
74 stars 21 forks source link

fix for big files sync #15

Closed sgaluza closed 8 years ago

sgaluza commented 9 years ago

If we read file into the buffer and the file is big (a few GBs) the ftpsync crashes. So no need to read the file with fs.readFile. jsftp can be used with just a filename as a parameter and with it the syncing works file with big files. Big files support would be a great feature for backup solutions (which I use it for:))

sgaluza commented 9 years ago

Hi, @evanplaice

Can you take a loot at the pull request? It's pretty straightforward..

evanplaice commented 8 years ago

Is this a limit to readFile. How does put differ?

Also, would you mind running the test to see if it breaks anything. The tests aren't very good due to the difficult of mocking a ftp server in JS.

To run them:

Note: you may need to run the test multiple times to sync all the files. When it's run locally, ftpsync fires off requests faster than the server can resolve them so requests get dropped.

It should synchronize the files from local to remote. To test incremental sync just delete a bunch of files from the remote directory before running the test. The sync should only sync the files you deleted.

sgaluza commented 8 years ago

readFile approach reads the whole file into memory and then copies it to remote folder, but put approach does a buffered/chunked copy.

I've tried the test you suggested. Everything completed successfully and files are copied from local to remote. BTW, probably it makes sense to add ftpd to dev dependencies in package.json.

sgaluza commented 8 years ago

I've rebased from a recent master and created a new PR for the issue. This PR can be rejected.

evanplaice commented 8 years ago

Right, streaming is the obvious choice.

Good point. I can add nodeftpd as a dep after I merge PR #23

evanplaice commented 8 years ago

I'm not sure if I'm missing something but ftpd should already be in the devDependencies. Have you rebased to the latest?

sgaluza commented 8 years ago

@evanplaice no issues with ftpd, I see it added to the new version of package.json

evanplaice commented 8 years ago

@sgaluza The dependency was added less than a month ago in b4e961d.

Anyway, thank you for your contribution.