Skip to content
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

Don't require using a file to pass nodes to build a derivation with all the system in a grid. #186

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tomprince
Copy link
Contributor

@tomprince tomprince commented Feb 7, 2022

Based on #166.

This adds a new argument to machine, which is the list of node names to build. This also changes argsFile to argsJSON, for passing the list of node names from go to nix without needing to write a file to disk.

I also added the ability to pass a grid as an attrset, rather than a path to a file with the grid. (I was intending to use this to pass a pre-evaluated nixpkgs in to multiple grids in a single evaluation, to save evaluation time, but it appears that doing so doesn't appear to have a significant impact on evaluation time.

Comment on lines +111 to +114
names' = if argsFile != null then
(builtins.fromJSON argsJSON).names
else
names;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to not need this logic. I filed NixOS/nix#6059 to be able to remove it.

@srhb srhb added triaged Discussed in-team, actionable needs-review Want to merge, team needs to test and review labels Mar 4, 2022
@srhb
Copy link
Contributor

srhb commented Mar 4, 2022

Deferring review here until prior PRs have been rebased and merged, sorry for the wait :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review Want to merge, team needs to test and review triaged Discussed in-team, actionable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants