purpleidea / mgmt

Next generation distributed, event-driven, parallel config management!
https://purpleidea.com/tags/mgmtconfig/
GNU General Public License v3.0
3.47k stars 308 forks source link

WIP discussion of function values to resources #716

Open purpleidea opened 11 months ago

purpleidea commented 11 months ago

Hey @gelisam I opened this PR (it's based of #704) to discuss passing function values to resources. In playing around first I hit an expected bug in the Into() function. This function pushes a bunch of mcl values INTO a golang struct (the resource). Of course the function part wasn't implemented.

So I started moving things around. And I have the import cycle of Txn <-> Types since I thought FancyFunc should maybe be part of the types package. Although it's a bit different because it has the unusual Txn aspect in there.

Before proceeding with a more complex refactor, I figured I'd check with you if you think this all makes sense or if you had a thought about how instead we'd pass in a simpler FuncValue to resources instead? (I don't expect they'll need the Txn, that's not the kind of functions we'd be passing to the resource I don't think.)

Cheers!

gelisam commented 10 months ago

Notes from today's meeting:

gelisam commented 10 months ago

I propose a new static analysis phase after type checking and before Expr.Graph(). The input is a mapping from lambda-bound name to their type, along with an annotation indicating whether each function type inside that type is timeless or timeful. The output is a similarly-annotated type for the Expr. We then check if in test "name" {func1 => expr}, the annotated type is a timeless function, and if it returns a function, that function is itself timeless, etc.

gelisam commented 10 months ago

Since you opened this PR, I guess this time I'll be the one pushing to a auxiliary branch and you'll be the one updating the official MR branch to incorporate my changes! I have pushed to gelisam/functions-to-resources. So far it's the same as master, because now that #704 has been merged, we should obviously start from the merged version.

gelisam commented 10 months ago

Could you post the examples and counter-examples of what counts as timeless, both as a reminder of what the goal is and so we can use them as tests?

purpleidea commented 10 months ago

I'll fix and throw away this branch. But if you'd prefer to have the PR branch, lmk.

Could you post the examples and counter-examples of what counts as timeless, both as a reminder of what the goal is and so we can use them as tests?

In short:

timeless:

timeful:

purpleidea commented 10 months ago

Here are the notes I took (may contain errors)

import "strconv"

$fn1 = func($x) { # pure! also good
    strconv.itoa($x)
}

$fn2 = if datetime.now() % 2 == 0 { # should be good! in fact, this has to work.

    func($x) { # middle point
        strconv.itoa($x)
    }

} else {

    func($x) {
        "hello"
    }

}

$fn3 = func($x) { # bad
    if datetime.now() % 2 == 0 { # this stream function makes things bad (it's also not pure)
        strconv.itoa($x)
    } else {
        "hello"
    }
}

$fn4 = func($x) {
    iter.map(...) # map is pure and okay here, as long as the input function is also.
}

$fn5 = func($x) { # bad!
    os.system("seq $x")
}

$fn6_prep = func($sam) { # this is ok or is it? -- the simple thing to do is that this is not allowed.
    func($x int) {
        $sam
    }
}

$fn6 = $fn6_prep(os.system("seq 10"))

$fn7_prep = func($sam) { # this is good

    if datetime.now() % 2 == 0 { # should be good! in fact, this has to work.

        func($x) { # middle point
            strconv.itoa($x)
        }

    } else {

        func($x) {
            "hello" + $sam
        }

    }

}

$fn7 = $fn7_prep("hello")

$fn8_prep = func($sam str) { # this is bad because it's simpler to do it that way and technically $fn8 is static but the insides are not timeless

    if datetime.now() % 2 == 0 {

        func($x) { # middle point
            strconv.itoa($x)
        }

    } else {

        func($x) {
            "hello" + $sam
        }

    }

}

$fn8 = $fn8_prep(os.system("seq 10"))

$fn9_prep = func($sam int) { # we actually agree this one is okay, but the reason it's okay is because $fn9 changes for each datetime.now() timeful change.

    if $sam % 2 == 0 {

        func($x) { # middle point
            strconv.itoa($x)
        }

    } else {

        func($x) {
            "hello"
        }

    }

}

$fn9 = $fn9_prep(datetime.now())

test "test1" {
    anotherstr => "hello",
    func1 => $fn3, // txn!
}

#
#   WHY DO WE WANT THIS?
#

import "prometheus"

# prometheus.connect is not pure because it has some sneaky side-effect somewhere...

$conn = func($x string) int {
    prometheus.connect($x) + 42 # not pure but timeless
}

test "test2" {
    func1 => $conn,
}

import "flagthing" # a built-in library

$flag1 = "flag1" # some arbitrary name

if flagthing.happened($flag1) {
    print "wow" {}
}

test "test2" {
    flagname => $flag1,
    func1 => $flagthing.start,
}
purpleidea commented 10 months ago

@gelisam Just wanted to double-check if this patch with interface and one implementation is basically what you were expecting? If so, LMK and I can complete it for other functions too.

gelisam commented 10 months ago

Looks good, yes!

gelisam commented 9 months ago

Next steps:

gelisam commented 9 months ago
purpleidea commented 7 months ago

Not the priority, but I was playing with this code and implemented a few extra for fun. I also pushed in your commits to have them here.

An interesting realization I had is that we should eventually once all have been implemented, be able to remove each mostly identical custom Stream() implementation with a single one that is common to all functions. Or most functions!