-
-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add nixops eval subcommand #1319
base: master
Are you sure you want to change the base?
Conversation
Before I review this, a few things: can you fixup the failures with And also if you could write a bit about what this is useful for, like what use cases you have for it -- etc. that'd be helpful! |
Yeah... working on it :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some feedback focusing mostly on the help messages. One thing this does is it sort of implicitly makes the structure of the evaluation's results an API. I know you already treat it like an API, but NixOps doesn't really make a claim about that right now.
I am not sure if we should do that, but want to still let you do what you need to do.
My thinking is two-fold:
- having a standard way for a NixOps network to become hydra-buildable is a great idea (Make networks buildable by Hydra #1320)
- if your goal is to have pre-deploy steps, then maybe a plugin and hooks would be a better approach, so a
nixops deploy
is doing the right thing?
I'm especially curious about your keyfile code there :)
what do you think?
nixops/__main__.py
Outdated
@@ -527,6 +527,16 @@ | |||
help="include the physical specification in the evaluation", | |||
) | |||
|
|||
subparser = add_subparser( | |||
subparsers, "eval", help="eval the given file is nix code in the network expression" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subparsers, "eval", help="eval the given file is nix code in the network expression" | |
subparsers, "eval", help="evaluate a Nix expression with the NixOps network as arguments" |
my proposed wording isn't quite right, but maybe something like this?
nixops/__main__.py
Outdated
subparsers, "eval", help="eval the given file is nix code in the network expression" | ||
) | ||
subparser.set_defaults(op=op_eval) | ||
subparser.add_argument("code", metavar="CODE", help="code") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subparser.add_argument("code", metavar="CODE", help="code") | |
subparser.add_argument("file", metavar="FILE", help="File containing a Nix expression") |
nixops/__main__.py
Outdated
subparser.set_defaults(op=op_eval) | ||
subparser.add_argument("code", metavar="CODE", help="code") | ||
subparser.add_argument( | ||
"--json", action="store_true", help="print the option value in JSON format" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"--json", action="store_true", help="print the option value in JSON format" | |
"--json", action="store_true", help="print the resulting value as an JSON representation of the abstract syntax tree" |
what does it print without --json
?
nixops/__main__.py
Outdated
subparser.add_argument( | ||
"--json", action="store_true", help="print the option value in JSON format" | ||
) | ||
subparser.add_argument("--strict", action="store_true", help="enable strict evaluation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a specific suggestion, but we might want to crib what the nix-instantiate manual says:
recursively evaluate list elements and attributes. Normally, such sub-expressions are left unevaluated
(since the Nix expression language is lazy).
Warning
This option can cause non-termination, because lazy data structures can be infinitely large.
I wonder why someone wouldn't use --strict --json
? Should --json
imply --strict
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, --json
will only work with --strict
if it's more than one level deep.
The use-case here was if you wanted to evaluate something but finely control the level of strictness and output Nix instead of JSON. For that you'd want lazy evaluation of functions, which is impossible if --strict
is hardcoded.
Given that this is supposed to be used by other tooling (that's why I don't pass a plain string but a file instead), it shouldn't be too confusing or hard to configure it for the specific usage.
@@ -893,6 +893,14 @@ def op_show_option(args): | |||
) | |||
|
|||
|
|||
def op_eval(args): | |||
with deployment(args) as depl: | |||
depl.evaluate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily to be fixed here, but it would be nice to think of a way for the deployment API to not require being careful about calling evaluate
.
This lets you pass a file to eval to
nixops eval
.With
nixops eval
you can evaluate nix code that has access to your deployment, so you can implement tooling aroundnixops
that depends on your actual configuration easier. This is currently not possible using justnixops show-option
, since that only accesses a single attribute.The simplest example would be to get the list of nodes:
Given this eval.nix file:
A more complex example would be generating keys that nixops depends on, using a custom
generator
attribute on the key:There are many more possibilities, but this really is a lot simpler than having to figure out the correct syntax for
eval-machine-info
yourself.