TritonDataCenter / node-workflow

Task orchestration, creation and running using NodeJS
MIT License
456 stars 66 forks source link

job attribute: workflow_uuid && workflow #135

Closed nopol closed 9 years ago

nopol commented 9 years ago

why are there two attributes? in both attributes is the uuid of the workflow stored

kusor commented 9 years ago

Sorry, I'm not following. The attribute workflow_uuid holds the UUID of the workflow for sure and, if I remember correctly, the attribute workflow, when present, points to the whole workflow object, which doesn't mean it needs to be stored, it's just there to be handy when manipulating jobs.

Is there any place where the whole workflow is stored as part of the job?, or a place where we're just pointing to the workflow uuid from job.workflow?

nopol commented 9 years ago

in the tests the workflow chain is stored in the chain and onerror attribute (arrays).

the workflow attribute is multible times set to the current wf_uuid. e.g.

runner.js:440

    function run() {
        // ...
        function retryJob(oldJob, callback) {
            var retryParams = {
                workflow: oldJob.workflow_uuid,
        // ...

as well as it is not optional as you just mentioned. the attribute is tested

workflow-factory.js:182

    function job(j, opts, callback) {
        var theJob = { execution: 'queued', chain_results: []};

        if (!j.workflow) {
            return callback('"j.workflow" is required');
        }
        // ...

and the specs speaks about the uuid should be stored in the attribute.

workflow-factory.js:169

    // - j - the Job object workflow and extra arguments:
    //   - workflow - (required) UUID of Workflow object to create the job from.

as well as on the test-files apt.test.js:405 apt.test.js:417 ...

perhaps I see a few things wrong. I got a little bit confused.

kusor commented 9 years ago

Yeah, but that's merely a way to refer to things into the workflow-factory, though. Exactly on that same function you mentioned, some lines below, you have this: theJob.workflow_uuid = wf.uuid, which makes pretty clear that it's the workflow_uuid attribute what we persist with the job in the backend.

A different thing is how we call the argument names here and there, which could be improved, of course, but I don't think it's a big deal right now, given no functionality is affected at all.

As far as what we're storing on the backend are the "foreign keys" as plain UUID strings, it's more than enough.