-
Notifications
You must be signed in to change notification settings - Fork 129
chore: add native deduplication to flake checks #330
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
base: master
Are you sure you want to change the base?
Conversation
this should fix [serokell#318](serokell#318) by deduplicating the flake paths with a Hashset
Sntx626
left a comment
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.
This does not de-duplicate all flake checks, just flake checks from having the same node provided multiple times...
What is the use-case for this?
|
Currently, if I deploy multiple nodes from a single repo (i.e. |
Sntx626
left a comment
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 see, in this case I partially misunderstood the purpose of this PR in my previous comment.
I tested this after applying #341 to be able to pass multiple deployment --targets.
It works as described, though it still evaluates the same flake for multiple hosts.
Fixes the problem described.
LGTM
|
Hey what exactly do you mean by evaluating? Evaluating the configuration of the particular host, i.e. the system config or doing the |
|
I'm getting multiple evaluations of the same flake ( I'm referring to the multiple evaluations that should also be able to be de-duplicated, since one evaluation per flake should result in all nodes. |
|
That is a misconception on your part. For each node/ profile the nixosConfiguration (via. outputs.nixosConfiguration.config.system.build.toplevel`) is evaluated in that step. As deploy-rs does not know anything about this configuration it cannot assume that the configuration is the same across the nodes/ profiles. |
this should fix #318 by deduplicating the flake paths with a Hashset