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
nixos home-assistant: a couple of fixes #36338
Conversation
configFile | ||
]; | ||
path = [ | ||
"/run/wrappers" |
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.
Can you mention ping
here?
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.
done
@@ -121,6 +120,19 @@ in { | |||
ReadWritePaths = "${cfg.configDir}"; | |||
PrivateTmp = true; | |||
}; | |||
restartTriggers = [ | |||
configFile |
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.
Good catch!
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.
Actually, thinking about it, shouldn't this be unnecessary becasue configFile
is mentioned in preStart
?
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. It should be unnecessary, since preStart
is regenerated if cfg.config
changes.
Can you rebase on staging so we can run the test? |
I've run the test locally against unstable (with the pytest-mock upgrade reverted), which was fine, but of course we can try against staging. |
a) set path to /run/wrappers so ping works b) restart if the configuration file changes c) run via a target so we can easily inject other components (config copier, appdaemon)
Ehhh...... What the what? Rebasing against staging didn't exactly go as planned... |
@peterhoeg I have rebased. We can wait with merging until the pytest-mock fix is in master. @GrahamcOfBorg test home-assistant |
Failure on aarch64-linux (full log) Partial log (click to expand)
|
Failure on x86_64-linux (full log) Partial log (click to expand)
|
I don't know why ofborg failed. I tested locally and it works fine. |
@GrahamcOfBorg test home-assistant |
@GrahamcOfBorg test home-assistant |
Success on aarch64-linux (full log) Attempted: tests.home-assistant Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: tests.home-assistant Partial log (click to expand)
|
@peterhoeg Feel free to backport to 18.03. |
Failure on aarch64-linux (full log) Attempted: tests.home-assistant Partial log (click to expand)
|
a) set path to /run/wrappers so ping works b) run via a target so we can easily inject other components (config copier, appdaemon) (cherry picked from commit 2859483)
Failure on x86_64-linux (full log) Attempted: tests.home-assistant Partial log (click to expand)
|
Motivation for this change
a) set path to /run/wrappers so ping works (what gets added is /run/wrappers/{bin,sbin})
b) restart if the configuration file changes
c) run via a target so we can easily inject other components (config copier, appdaemon)
Cc: @dotlambda
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)