Skip to content

Proof of concept for function interpolation. - #48

Closed
hakoerber wants to merge 14 commits into
madduck:masterfrom
hakoerber:master
Closed

Proof of concept for function interpolation.#48
hakoerber wants to merge 14 commits into
madduck:masterfrom
hakoerber:master

Conversation

@hakoerber

Copy link
Copy Markdown

See #47.

This is only a proof of concept and implements a simple print() function.

Hannes Koerber added 3 commits July 25, 2015 13:56
This makes it easier to add other kinds of interpolation later on.
Everything inside $[...] blocks is treated as a function. Right now,
only print() is supported, which just concatenates and all its arguments to a string.
So for example,

    key: $[print(a, b, c)]

would resolve to

    key: "a b c"
@madduck

madduck commented Jul 25, 2015

Copy link
Copy Markdown
Owner

This looks very interesting. The only thing I'd say is that the functions should be provided (registered) as proper Python objects, rather than string-compared within a function. What do you think about that?

@hakoerber

Copy link
Copy Markdown
Author

Good idea, I will look into it.

@hakoerber

Copy link
Copy Markdown
Author

Ok, I think this is much more readable and can be extended easily if additional functions are needed.

Hannes Koerber added 3 commits July 25, 2015 16:58
This is to allow python dict access in the function, like node['ip'].
This is needed because functions in a node definition might access other nodes.
This is a bit hacky. Parameter expansion inside function interpolation does not work, so $<aggregate(node[name] == ${some:value}, node[name])> fails.
@hakoerber

Copy link
Copy Markdown
Author

So, this is a first working version. I resorted to always doing a complete inventory, even if only simple nodeinfo is requested. At the end of an inventory run, the functions are expanded, and each function gets the complete inventory as a parameter so it can query other hosts.

Things that are missing:

  • Parameter expansion inside a function does not work. It fails with a KeyError in storage/memcache_proxy.py:_cache_proxy for some reason.
  • Circular dependencies are not resolved, which would result in infinite recursion.

Hannes Koerber added 6 commits July 28, 2015 03:34
Instead of returning a list of all extracted values, it now returns a dict mapping the nodename to the extracted values.
This did not work beforehand because functions and references were
parsed at the same time, but evaluated later. This parsing failed when a
reference was nested in a function.

To solve this, function evaluation *and parsing* are now done after the
whole inventory is available. This means that all references are already
expanded, so neither the function nor the reference parsing need to be
altered.
  - Rename RefValue classes to something more specific:
    - ReferenceStringParameter and ReferenceStringFunction represent
      strings that contain references (parameters and functions
      respectively)
    - ReferenceParameter and ReferenceFunction represent references in
      the classes above, and can be interpolates/executed

  - Revert the behaviour that functions and parameters were parsed at
    the same time, but functions "saved" until the whole inventory was
    available. Now, functions will be parsed and executed after a
    complete inventory run.
@hakoerber

Copy link
Copy Markdown
Author

OK, I reworked it a bit:

  • I added a bit of documentation for function interpolation.
  • By completely separating parameter and function interpolation, with the latter being done at the end of the inventory run, parameters can now be used in functions.
  • Circular dependencies are avoided by simply not interpolating functions in results from other functions.

The tests still pass, but I'm too lazy to add new ones for the new code. 😄

If you are still interested in the code, I would rebase and clean up the new commits and send a new pull request.

@lottspot

lottspot commented Aug 6, 2016

Copy link
Copy Markdown
Collaborator

So I think that this feature would be superseded by #56 if #56 is merged. From what I can tell, mutators would be quite capable of absorbing the use cases for function interpolation.

@hakoerber

Copy link
Copy Markdown
Author

Yup, #56 looks cleaner than this approach. Guess I can close this.

@hakoerber hakoerber closed this Aug 9, 2016
AndrewPickford pushed a commit to AndrewPickford/reclass that referenced this pull request Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants