restify / conductor Goto Github PK

Allow a Model to depend on other models by supplying an array of dependencies that will be checked at runtime. The deps list would be a set of Strings, which would just do a quick check to see if the Model is already on the request or not. If it's not, it will warn or message an error so that developers can more easily see why their model is failing.

Rename lifecycle methods to be more explicit about when they are happening. before() and after() may not be immediately obvious.

createModel is exported twice.

https://github.com/restify/conductor/blob/master/lib/index.js#L126-L127

As the sortNumericalKeys function uses the built in Array sort, it should return -1, 0 or 1 (or anything less than or greater than 0 to indicate -1 and 1). The current implementation returns a true / false value which will not return the desired sort ordering.

The current implementation is can be found here:

Trying to set a property in a props function, when a dependency didn't return an object causes this error which is hard to find the root cause. There should be a handler around calling the props function, which determines if this._props is undefined or not, and throws an error so users can correct the issue.

TypeError: Cannot set property 'property' of undefined
    at Object.module.exports.rc.createConductor.props [as propsFn] 
    at Conductor._mergeDeps (/restify-conductor/lib/Conductor.js:256:35)

It would be amazing to have a way to visually represent any arbitrary conductor's dependency tree, and the resulting props / config that would happen on startup. Thanks to @clarle for the idea.

Model life cycle methods are currently sync, consider making them async.

Restify allows you to pass an object when creating a new route, this should be allowed when using installConductor which creates the route behind the scenes.

This would be a simple shorthand for creating a conductor with the minimum amount of information required. Passing in the name as the optional first parameter would reduce the boilerplate for creating a new Conductor.

If a set of handlers for a bucket isn't properly flattened, the error message returned is rather cryptic, provide a bit more help by warning that the handler is an array instead of a function.

TypeError: object is not a function
        at runHandlers (/Users/mransdell/src/shakti/app/node_modules/restify-conductor/lib/handlers/run.js:105:21)
        at Object.func (/Users/mransdell/src/shakti/app/node_modules/restify-conductor/node_modules/vasync/lib/vasync.js:246:12)
        at Immediate._onImmediate (/Users/mransdell/src/shakti/app/node_modules/restify-conductor/node_modules/vasync/lib/vasync.js:213:20)
        at processImmediate [as _immediateCallback] (timers.js:358:17)

https://github.com/restify/conductor/blob/master/lib/handlers/run.js#L104

Tests currently export an instance of a model, so model mutation causes persistence between test cases. These files should export factories instead, allowing new models to be used per test case.

Mostly for metrics / reporting, but useful context for applications.

Currently, the only client that can be used is a restify-client and it's auto-configured per host. It would be helpful to allow developers to create their own clients (and manager their pools) and expose a setClient method, that takes in a client and the host it should be used for.

Early feedback that being explicit may be more helpful.

Hi,

Thank you all for all the work on this.

I've been working on dynamic route definitions and would like to give conductor a try. Essentially I need to be able to add, remove routes or swap handler stacks for an existing route. I've tried doing something along the lines of:

    // Adding a dynamic route
    server['get']({
      name: 'uniqueName',
      path: '/some/route/path',
    }, [handlerA, handlerB, handlerC]);

    // Detecting, say, a delete in the db then doing...
    server.rm(endpoint.name);

    // Later perhaps another change comes in with the same path but different handlers
    server['get']({
      name: 'uniqueName', // this often is the same as before (ie: editing handlers)
      path: '/some/route/path', // this often is the same as before (ie: editing handlers)
    }, [handlerX, handlerY, handlerZ]); // note these are different handlers

In Restify (not using conductor yet) I'm inspecting server.routes and I DO see the effect of removing a route. The problem is I get the following when I try curl -is "http://localhost:8080/some/route/path":

/Users/lgomez/Projects/proxy/node_modules/restify/lib/plugins/audit.js:102
            req_id: req.getId(),
                        ^

TypeError: req.getId is not a function
    at Server.audit (/Users/lgomez/Projects/proxy/node_modules/restify/lib/plugins/audit.js:102:25)
    at emitThree (events.js:97:13)
    at Server.emit (events.js:175:7)
    at emitRouteError (/Users/lgomez/Projects/proxy/node_modules/restify/lib/server.js:202:16)
    at onRoute (/Users/lgomez/Projects/proxy/node_modules/restify/lib/server.js:762:17)
    at Router.find (/Users/lgomez/Projects/proxy/node_modules/restify/lib/router.js:419:9)
    at Server._route (/Users/lgomez/Projects/proxy/node_modules/restify/lib/server.js:747:21)
    at routeAndRun (/Users/lgomez/Projects/proxy/node_modules/restify/lib/server.js:705:14)
    at Server._handle (/Users/lgomez/Projects/proxy/node_modules/restify/lib/server.js:725:9)
    at Server.onRequest (/Users/lgomez/Projects/proxy/node_modules/restify/lib/server.js:326:14)

The only way I have been able to avoid that error is by restarting the server but that will not be an option in prod.

So, in the readme for Conductor I see one of the goals being to decouple the handler stacks from the routes themselves. If Conductor can help, how would you remove a route completely or, even better, replace it?

Thank you!

It could be frozen the same way, but just allow for statically creating props.

Meant to handle legacy migration.

It would be very helpful to provide a mechanism to gets all the models that are in a "bucket" without having them automatically fetched.

A proposal would be to change the signature of buildModelsWrapper in such a way that it will accept a function to call in the forEachParallel call and default to the current one if it's not passed in. This will allow developers to handle fetching of models or any other customizations that need to be done without re-implementing the entire process.

Return an object of the models keyed by name if they are there, null if not.

Would be nice to get the resolved url of the conductor or at least the url of where it was installed, as a property.

Since conductor is doing http client requests for the end user, need to expose metrics information more easily from the client. Maybe lifecycle hooks for the client itself?

In order to inherit the methods / properties on a Model, one has to "create" one then inherit from it's prototype. This seems inefficient and unnecessary. Perhaps a separate module for models would enable inheritance as well as contribution back to the set of models.

Is there value in using the model/client independently of conductor? They could live together in their own module 'restify-rds' (remote data source).

Each time a conductor is run, keep a log of the entire run of:

• conductors sharded to/from
• what handlers were run, what handlers were left on the table per sharded conductor
• the actual handlers run (which is probably replicated by restify audit log)

There are times when you want to alias a model when it's set in the models for a conductor. It would be great to have the ability to alias a model (i.e. context for a general model).

I propose we make the function signature:

setModel(req, model, [modelName])