Support for custom "wrappers"

[ravi]

Per the documentation, there are convenience wrappers for common commands like ls. It would be very useful if this feature can be extended such that we can add custom wrappers, and even share them:

var result = transport.getuid('mylogin');
if( result.code === 0 && result.stdout !== 10000 )
    transport.abort("User 'mylogin' already exists with the wrong uid: " + result.stdout);

I realise stdoutin the above is a string, it contains a newline, etc, but I am ignoring that in the interest of keeping example simple. I hope you get what I am driving at.

Since this is a somewhat significant and philosophical issue, I am asking first if this is an interesting direction for the project, rather than sending a PR.

[pstadler]

Right now you can extend transports like this:

Transport.prototype.getuid = function(args, opts) {
  opts = this._parseOptions(opts);
  return this.__exec('getuid', args, opts);
};

This is far from optimal, what would your solution look like?

[ravi]

That's good enough I think, but the question is how is that (the prototype) best exposed? Some conventions would help: how do extensions access the transport prototype, where are they stored, etc. Considering only *nix for the sake of simplicity, maybe Flightplan could look at $FLIGHTPLAN_LIB/extensions and require() every file there, passing as args the Transport prototype... or some such.

[pstadler]

It's not a good idea to store user data into a folder controlled by npm, or introducing an obscure env variable.

What if you would just extend your Flightplan class to fit your needs and then in flightplan.js you simply require your modified version. Example:

var Flightplan = require('./lib/deployment/flightplan'); // extended class

var plan = new Flightplan();
//...

[ravi]

I agree on not messing with npm controlled directories (hence the suggestion of an env variable). What you suggest is very similar to what I am doing right now, actually. But I was hoping for a solution that permits easy sharing of such extensions, which is really where the benefit lies... if I can find extensions/recipes from the Flightplan ecosystem, that would save me time (and vice versa with my extensions).

BTW, marginally related to this, I also have a crude mechanism to "include" flightplans in others, by exporting the flight in module.exports. Not sure if you have any plans such a feature, and if you do, +1 on that from me :-).

[pstadler]

So you'd like to see some kind of tooling ecosystem like gulp or grunt has? Why not. I'm planning to refactor Flightplan at some point, because the current codebase is far from optimal. I try to remember this discussion and make a good interface for extensions and plugins.

Do you have any input regarding splitting the codebase into smaller pieces or how you'd like to interact with the API for writing plugins?

[ravi]

Sorry missed the above comment. I have not looked into the code much so unfortunately do not have any suggestions on what structural changes can be made. In terms of the API for writing plugins, even a somewhat clean way of exposing the prototype might do... the real usability issue, I feel, will be in how easy it is to share and deploy these plugins.

[pstadler]

Hey @ravi

If you're still interested in this subject, please make a quick proposal in the form of code snippets how you'd like to write extensions and how to load them as a developer.

Thanks
Patrick

[akloeber]

Hi @pstadler
I'm working on a pull request for that. What do think about registration with the following synopsis registerCommand(cmdName, cmdDef[, defaultOptions]) that can be used like so:

plan.registerCommand('mkpdir', 'mkdir -p');
// or
plan.registerCommand('mkpdir', ['mkdir', '-p']);

In the same way I want to extend argument passing so that the when those are given as an array automatic shell escaping occurs (through https://www.npmjs.com/package/shell-escape) which can make scripts smore readable:

  var dirWithBlanks = '/var/foo bar/bazz'
  plan.mkdir('-p "' + dirWithBlanks + '"');
  // can be written as
  plan.mkdir(['-p', dirWithBlanks]);

This would be backwards compatible as it is only triggered if extension has type array.

What do you think?

[pstadler]

Hi @akloeber

1. plan is an instance of Flightplan. You need to register commands on the Transport class.
2. how to handle commands which are only available for either SSH or Shell but not both?
3. newly introduced commands are not following the API of existing commands
4. what's the advantage over simply writing transport.exec('mkdir -p "dir"')?

[akloeber]

@pstadler:

-> 1.
Well, so far flightplan is the only exposed object, so there is no real alternative to that without changing the API of the entire package. Maybe the flightplan object could expose some kind of config handle to do that kind of global configuration.
-> 2.
My intention was that registerCommand could be used to define a custom alias for a command (optionally with default options) that is often used, so that the overall deployment code becomes more readable and provided options can be managed centrally at one point (at the location where the command is defined). If the command can only run locally or remotely of course there will be a command not found error but that is in the responsability of the user that defines the alias. Even now there is no guarantee that each shell supports all of the build-in commands coming from commands.js (although of course the major ones do). Or there may be small differences in the supported arguments (see hostname -n command for example which works on Linux but not on Mac). Even then the possibility of registering own commands at a central place would support workarounds for that at a single place.
-> 3. and 4.
The API for using a command should be the same as the current ones. Providing arguments as an array with automatic shell escaping should be optional and providing the entire command string should still be supported. It's only for readability, compare the following:

var source = '/var/foo bar/bazz';
var target = '/var/bar foo/bazz';
plan.exec('mv "' + source + '" "' + target + '"');
// can be written as
plan.exec(['mv', source, target]);

If source/target may contain additional special characters (like &) it may be necessary to do something like that:

var esc = require('shell-escape');
plan.exec('mv ' + esc(source) + ' ' + esc(target));

In my opinion that could be handled nicely within flightplan (while also staying backwards compatible) and it would improve the overall readability/maintainability of the scripts.

[ravi]

With apologies for not fully reading the comments above before I write the below: IMHO custom commands/wrappers should be auto-discovered by Flightplan, such as in the example above. Wrappers should be able to do complex operations not just run one single command.