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

Question about download/upload API #58

Closed
warpdesign opened this issue Feb 22, 2019 · 12 comments
Closed

Question about download/upload API #58

warpdesign opened this issue Feb 22, 2019 · 12 comments

Comments

@warpdesign
Copy link

warpdesign commented Feb 22, 2019

I am porting my ftp client so that it's now using basic-ftp instead of node-ftp.

Everything is working ok so far but it seems the download API differs in that node-ftp has a get API that's basically:

client.get(remotePath, callback(err, readableStream))

So I'm getting a readableStream that I can pipe through a writetableStream I create myself.

The good thing about that is that I can even download something from an FTP server and directly pass it through to the put method of another FTP client with the put API:

client.get(remotePath, (err, readableStream) => {
  if (!err) {
    client2.put(readableStream, remotePath);
   }
});

With basic-ftp, the download API needs a writableStream instead of a readableStream so this isn't possible anymore.

I briefly looked at the source code but couldn't find an easy to get a readableStream when downloading something. Is it possible without much work?

@warpdesign
Copy link
Author

After some tests, it seems it's as easy as using ftp.dataSocket as a readable stream.

Any chance of having an API for downloading that doesn't require a writeStream (that doesn't call ftp.dataSocket.pipe(destination) and resolves with ftp.dataSocket?

@patrickjuchli
Copy link
Owner

The central use-case for this library is to download data to a file. For that, you use "fs.createWriteStream()", hence the need for a writable stream. Alternatively you can also provide stdout. When getting a directory listing, data is written into a string for example.

If you want to read again from the stream, you just provide a duplex or transform stream.

Be careful with piping data yourself from dataSocket – I'm not sure how reliable that actually is. Isn't it possible that you'll start piping too late and miss some data? Not sure. Outside of the scope of this library, I can't provide support for this as it reaches into internals.

@patrickjuchli
Copy link
Owner

patrickjuchli commented Feb 22, 2019

So downloading a file looks like this:

const dest = fs.createWriteStream("myLocalFile");
const response = await client.download(dest, "myRemoteFile");

// or logging to console:
const response = await client.download(process.stdout, "myRemoteFile");

@warpdesign
Copy link
Author

I think passing a readStream would allow both use cases:

  • you may pipe it to a writeStream created using fs.createWriteStream() in order to write it to a file
  • or pass it to another function that expects a readStream as a source

As it is now, I'm stuck if I want to call a method that expects a readStream.

@warpdesign
Copy link
Author

So downloading a file looks like this:

const dest = fs.createWriteStream("myLocalFile");
const response = await client.download(dest, "myRemoteFile");

// or logging to console:
const response = await client.download(process.stdout, "myRemoteFile");

What if I want to send the client.download() stream and pass it to client2.upload()?

@patrickjuchli
Copy link
Owner

You are definitely not stuck. Again, you need to pass a writable stream that is also readable. Here is an example with Transform stream from "streams".

const transform = new Transform({
  transform(chunk, encoding, callback) {
    // Do anything here
    callback(null, chunk);
  }
});
await client.download(transform, "bla.json")
transform.pipe(process.stdout)

Passing a readable stream won't work because the download function needs to write to it, not read from it. You probably want the download function to return a readable stream. But it doesn't do that because it returns the Promise when the whole operation is done. And this doesn't just depend on whether the data transfer is done but also if the server confirmed it.

@patrickjuchli
Copy link
Owner

patrickjuchli commented Feb 22, 2019

Here is a complete example that downloads a file from one client while compressing and uploading it on the fly to another:

const ftp = require("../dist/index");
const zlib = require('zlib');

example();

async function example() {
    const c1 = new ftp.Client();
    const c2 = new ftp.Client();
    try {
        await c1.access({
            host: "somewhere",
            user: "someone",
            password: "ohno",
            secure: true,
        });
        await c2.access({
            host: "somewhereelse",
            user: "someone",
            password: "ohno",
            secure: true,
        });

        const zipper = zlib.createGzip();
        const promise1 = c1.download(zipper, "bla.json")
        const promise2 = c2.upload(zipper, "bla.zip")
        // Download and upload now happen at the same time, we wait until
        // both of them are finished.
        await Promise.all([promise1, promise2])
    }
    catch(err) {
        console.dir(err);
    }
    c1.close();
    c2.close();
}

@warpdesign
Copy link
Author

warpdesign commented Feb 22, 2019

Oh, thanks, I hadn't thought about transforms!

That said, I definitely cannot wait for the download to be over:

const transform = new Transform({
  transform(chunk, encoding, callback) {
    // Do anything here
    callback(null, chunk);
  }
});
// don't wait for the download to be over before using transform.pipe()
client.download(transform, "bla.json")
transform.pipe(process.stdout)

Otherwise both transfers won't happen in parallel.

@patrickjuchli
Copy link
Owner

Yep, but the complete example above is parallel.

@alemens
Copy link

alemens commented Apr 24, 2019

This code below it works, but I commented client.close(); to avoid the error (node:42413) UnhandledPromiseRejectionWarning: Error: Client is closed

and if I use await server gives time out @warpdesign

So I do not know where to use client.close();

@patrickjuchli could you help here?

const ftp = require("basic-ftp");
const { Transform } = require('stream');
const fs = require("fs");

downloadFtp()

async function downloadFtp() {
  const client = new ftp.Client();
  client.ftp.verbose = true;

  try {
    await client.access({
      host: "host",
      user: "user",
      password: "password",
      secure: false
    });
    await client.ensureDir("OUT");
    const listOut = await client.list("OUT");
    var lastRemoteOut = listOut.pop().name;

    const lowCaseTransform = await new Transform({
      transform(chunk, encoding, callback) {
        this.push(chunk.toString().toLowerCase());
        callback();
      }
    });

    client.download(lowCaseTransform, lastRemoteOut);
    lowCaseTransform.pipe(fs.createWriteStream("./public/OUT.txt"));
  }
  catch (err) {
    console.log(err);
  }
  // client.close();
}

exports.downloadFtp = downloadFtp;
Thank you!

Oh, thanks, I hadn't thought about transforms!

That said, I definitely cannot wait for the download to be over:

const transform = new Transform({
  transform(chunk, encoding, callback) {
    // Do anything here
    callback(null, chunk);
  }
});
// don't wait for the download to be over before using transform.pipe()
client.download(transform, "bla.json")
transform.pipe(process.stdout)

Otherwise both transfers won't happen in parallel.

@alemens
Copy link

alemens commented Apr 24, 2019

Solved!

const ftp = require("basic-ftp");
const { Transform } = require('stream');
const fs = require("fs");

downloadFtp()

async function downloadFtp() {
  const client = new ftp.Client();
  client.ftp.verbose = true;

  try {
    await client.access({
      host: "host",
      user: "user",
      password: "password",
      secure: false
    });
    await client.ensureDir("OUT");
    const listOut = await client.list("OUT");
    var lastRemoteOut = listOut.pop().name;

    const lowCaseTransform = await new Transform({
      transform(chunk, encoding, callback) {
        this.push(chunk.toString().toLowerCase());
        callback();
      }
    });

    lowCaseTransform.pipe(fs.createWriteStream("./public/OUT.txt"));
    await client.download(lowCaseTransform, lastRemoteOut);
  }
  catch (err) {
    console.log(err);
  }
  client.close();
}

exports.downloadFtp = downloadFtp;

@aurelien0intent
Copy link

Hi @patrickjuchli, does Transform stream work with downloadTo API ?

When I try to use it, transfer never trigger the "onDataDone" function. But when I use Writable stream, it's work.

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

4 participants