Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

looking for a handful of testers #4

Closed
ctalkington opened this Issue Jan 2, 2013 · 19 comments

Comments

Projects
None yet
5 participants
Owner

ctalkington commented Jan 2, 2013

i'm looking for a handful of testers that work with archives and node on a regular basis and would be willing to help test new features. i really want to make this module the go-to for all things archiving in node.

bf commented Mar 1, 2013

I just tested the new version with my existing codebase, and I must say it works very vell for creating zip files. I have now thrown away my local changes and switched back to the v0.4 branch. Thanks for the improvements!

Owner

ctalkington commented Mar 1, 2013

glad to hear! it was a major re-factor to move away from its zipstream roots.

Was using your implementation of zipstream before to stream files from S3 to a ZIP, without storing anything on disk. Since the new version of Node that just got released, zipstream broke, so I had to look for another solution. This module is awesome. Just replaced zipstream by this and it works super well.

Owner

ctalkington commented Mar 26, 2013

glad to hear. there is more to come, just been tied up with paying work this month.

matoho commented May 9, 2013

@jValdron, I would be very interested to know more about your approach to create a zipstream from S3. I tried using it together with the awssum module and ended up with the following code:

var fmt = require('fmt'),
    amazonS3 = require('awssum-amazon-s3'),
    fs = require('fs'),
    archiver = require('archiver'),
    config = require('../config.js');


exports.download = function(req, res){

  res.setHeader('Content-disposition', 'attachment; filename=test.zip');

  //var output = fs.createWriteStream(__dirname + '/example-output.zip');
  //console.log(__dirname + '/example-output.zip');
  var archive = archiver('zip');

  var s3 = new amazonS3.S3({
    'accessKeyId'     : config.aws_key,
    'secretAccessKey' : config.aws_secret,
    'region'          : amazonS3.US_EAST_1
  });


  archive.on('error', function(err) {
    throw err;
  });

  archive.pipe(res);

  for (var i = 1; i < 4; i++) {


    s3.GetObject({BucketName:config.aws_bucket,ObjectName:'hallo/abc'+i+'.jpg'}, { stream : true }, function(err, data) {
      archive.append(data.Stream, {name:'abc'+i+'.jpg'});
    });

  };

  archive.finalize(function(err, written) {
    if (err) {
      throw err;
    }

    console.log(written + ' total bytes written');
  });


};

Although, I can't get it to work as I always get the "write after end" error. As I am very new to node and streams I would be very happy if you could give me some tipps how to get this to run.

Owner

ctalkington commented May 10, 2013

@maxgoesup in this case you would need to wrap the s3.GetObject using a step or async module. basically you end up calling finalize before anything is actually added since it's waiting on s3 to append. that's what the "write after end" generally means.

matoho commented May 10, 2013

Yes, that's what I thought. Do you have any suggestions how I could go about wrapping the s3.GetObject into an async module?

Owner

ctalkington commented May 10, 2013

I generally use the async module.

https://github.com/caolan/async

being a for style you may want to try this method:

https://github.com/caolan/async?source=c#whilsttest-fn-callback

matoho commented May 10, 2013

Thank you this has helped me a lot - async looks like a great module. Now I also understand the problem of synchronous loops better. Sadly, I still receive this error. But if I understood correctly the files should be added asynchronously now right? So frist the zip gets served to the client and then the files are appended afterwards in asynchronous manner? Here is my new code:

...

  archive.pipe(res);

  var i = 1;

  async.whilst(
    function () { return i < 4; },
    function (callback) {
        s3.GetObject({BucketName:config.aws_bucket,ObjectName:'hallo/abc'+i+'.jpg'}, { stream : true }, function(err, data) {
          archive.append(data.Stream, {name:'abc'+i+'.jpg'});
        });
        i++;
    },
    function (err) {
        if (err) {
          throw err;
        }
    }
  );
...

Is that a wrong implementation?

Owner

ctalkington commented May 10, 2013

you would want to call finalize if no error in that last function. otherwise i believe that will work for the way your using it.

matoho commented May 10, 2013

Yay, it really worked. At least the download started. It just gets stuck now after appending the first file and it won't continue unless I quit the server. However I will try to solve it tomorrow and post my solution if I can get it to run. Thanks for the kind help so far!

Hey @maxgoesup, personally, I knox along with this awesome archiver module. Here is a sample code with knox, I removed error checks:

    // Load up the ZIP library and create a new archive.
    var archive = zip.createZip({ 
        level: 1 
    });

    // Set the content-type and the filename for the ZIP.
    res.contentType('application/octet');
    res.header('Content-Disposition', 'attachment; filename="name.zip"');

    // Pipe out the ZIP as the response.
    archive.pipe(res);

    // Get the file on S3.
    s3.getFile('path-on-s3', function(err, s3res){

        // Add the file to the ZIP.
        archive.addFile(s3res, {
            name: 'name-of-file-in-zip'
        }, function(){

           // Close the ZIP.
           archive.finalize();

        });

    });

    // When the ZIP ends, then we end the response too.
    archive.on('end', function(){
        res.end();
    });

matoho commented May 11, 2013

@jValdron , thanks a lot for your code. It works very well for zipping up one single file from S3, but if I want to zip and download an arbitrary list of files I had to go with the async approach @ctalkington was refering to. Now I finally had some time to fix my code and found a nice working solution with the async.forEachSeries function. Here is my complete code:

var fmt = require('fmt'),
    amazonS3 = require('awssum-amazon-s3'),
    fs = require('fs'),
    archiver = require('archiver'),
    async = require('async'),
    config = require('../config.js');


exports.download = function(req, res, next){

  res.contentType('application/octet');
  res.header('Content-Disposition', 'attachment; filename="name.zip"');


  var archive = archiver('zip');

  var s3 = new amazonS3.S3({
    'accessKeyId'     : config.aws_key,
    'secretAccessKey' : config.aws_secret,
    'region'          : amazonS3.US_EAST_1
  });


  archive.pipe(res);

  var downFiles = ['abc1.jpg','abc2.jpg','abc3.jpg'];
  async.forEachSeries(downFiles, function(downFile, callback) {
      console.log(downFile);
      s3.GetObject({BucketName:config.aws_bucket,ObjectName:'hallo/'+downFile}, { stream : true }, function(err, data) {
          archive.append(data.Stream, {name:downFile});
          callback();
      });
  }, function(err) {
      if (err) return next(err);
      archive.finalize(function(err, written) {
          if (err) {
            throw err;
          }

          console.log(written + ' total bytes written');
      });
  });



  archive.on('error', function(err) {
    throw err;
  });

  archive.on('end', function(){
    res.end();
  });


};

Thank you both for your help and the development of this great module!

Owner

ctalkington commented May 11, 2013

glad you worked it out, I think in the future I'm going to add a finalized event so one can go all events vs mixed.

👍 archiver is working great so far! I'm having trouble with paused file streams at the moment, but I imagine I'll get to the bottom of it eventually

Owner

ctalkington commented Sep 3, 2013

@mikermcneil glad to hear. its getting more mature with each release and the more people using it the better it is for everyone as we can catch any edge cases a lot faster. v0.5 will bring some helpers for things like targz and hopefully a way to better track pipe so that finalize() callback actually happens on the destination's end event instead of archiver's.

@ctalkington very cool!

Owner

ctalkington commented Oct 6, 2013

going to need some testers for v0.5.0 in next week or so. changes coming to how archive modules are structured and loaded. also looking to implement an output filter that will allow one to filter the archive contents before archiver outputs them to its readable side. this should open the door for internal gzip etc.

Owner

ctalkington commented Jan 11, 2014

v0.5.0 has been released. bring on the bug reports!

@mortengf mortengf referenced this issue in Scripler/scripler Sep 7, 2014

Closed

Downloading EPUB sometimes fails #391

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment