prosociallearnEU / cf-nodejs-client

A Cloud Foundry Client for Node.js
https://prosociallearneu.github.io/cf-nodejs-client/
Apache License 2.0
17 stars 41 forks source link

added support for passing in a stream #176

Closed jsloyer closed 8 years ago

jsloyer commented 8 years ago

@jabrena this is what I was talking about. Checks to see if a string is passed in, if its a string assumes its a zip file like the library previously did, if its not a string, assumes its a stream then and calls toBuffer().

jabrena commented 8 years ago

It is better if we maintain the method upload in the same way and we create a new method with the name: uploadByStream with the contract:

uploadByStream (appGuid, stream, async)

It could be interesting if request accept streams. The final idea for upload and the new method uploadByStream could be the usage of request dependency. If you like, we could create some test case to use request to upload a file and a stream.

jabrena commented 8 years ago

anyway, thinking about this requirement. if a developer download a project, it is possible to fetch a git repository as a stream to zip later as a stream?

jsloyer commented 8 years ago

@jabrena uploaded a new commit with a function called uploadByStream

jabrena commented 8 years ago

Yes, now exist a preliminar function with stream support but it is necessary to add test cases. For test case, my question is what is the scenario of usage. The initial method is generic. The developer pass a location of a file and the library upload the zip to a CF endpoint. What is the usage of a stream? I only see a clear scenario to test the CF infrastructure creating some app with a structure generated in memory. For other uses purpose, for example clone a git project and pass the stream, I am not sure if it is possible. Other scenarios to use? Your initial idea is to avoid zipping a folder, but I think that the scenario only avoid one action so my question is about if you see other scenarios to use a stream. I am trying to see some way to download in memory a project to stream to the new method in some way, but googling, at the moment, I don't see any reference.

jabrena commented 8 years ago

I made a request in nodegit: https://github.com/nodegit/nodegit/issues/988

jsloyer commented 8 years ago

This is the function Im using to zip a folder.

var archiver = require('archiver');
var Streams = require('memory-streams');
var readline = require('readline');
var stream = require('stream');

public zipApp(filePath) {
        return new Promise(function (resolve, reject) {
            var stats = fs.statSync(filePath);
            if (stats.isDirectory()) {
                var archive = archiver('zip');
                var writer = new Streams.WritableStream();

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

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

                archive.pipe(writer);

                var srcFiles = ['**'];

                fs.stat(path.join(filePath + '/.cfignore'), function (error) {
                    if (error instanceof Error) {
                        archive.bulk([
                            { expand: true, cwd: filePath, src: srcFiles, dest: ''}
                        ]).finalize();
                    }
                    else {
                        var inStream = fs.createReadStream(path.join(filePath + '/.cfignore'));
                        var rl = readline.createInterface(inStream, new stream);
                        rl.on('line', function(line) {
                            line = line.trim();
                            if (line.indexOf('#') === -1 && line !== '') {
                                srcFiles.push('!' + line);
                            }
                        });
                        rl.on('close', function() {
                            archive.bulk([
                                { expand: true, cwd: filePath, src: srcFiles, dest: ''}
                            ]).finalize();
                        });
                    }
                });
            }
            else {
                resolve(filePath);
            }
        });
    }
jsloyer commented 8 years ago

@jabrena I dont think you can assume everyone will have a git project. It can be any arbitrary folder that needs to be zipped.

jabrena commented 8 years ago

So the benefit to pass the stream is to avoid create a zip in file system but previously, the app exist. It could be an interesting scenario:

A developer execute npm deploy; deploy is a task to lint, test and deploy the source to a CF instance. Using this technique, it is not necessary to create in filesystem the zip and later remove.

It is a clear benefit.

Tomorrow, I will check if it is possible to upload with "request library" a stream.

jabrena commented 8 years ago

Related issue: https://github.com/request/request/issues/2159

jsloyer commented 8 years ago

Yeah. You can fetch a git repo in memory and then pipe it to a stream and then pass it to the the upload function. The stream is honestly the most flexible input to the upload function. Is your worry that we aren't including something? Integrating this as is with the stream support solves all the use cases. Zip file creation on disk is not what we want to be doing. If you go with your scenario above you create a zip file on zip for the git project and then you have to delete the zip file at some point, no one wants extra zip files created on their system or leaving them around.