nixos ddclient: support multiple domains and run via systemd timer #36864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Success on x86_64-linux (full log) Attempted: ddclient Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: ddclient Partial log (click to expand)
|
peterhoeg
changed the title
bjornfor
reviewed
Mar 13, 2018
| @@ -25,7 +26,12 @@ buildPerlPackage rec { | |||
| ''; | |||
|
|
|||
| installPhase = '' | |||
| runHook preInstall | |||
There was a problem hiding this comment.
Perhaps it's better to change installPhase to postInstall, so the hooks don't have to be invoked manually?
There was a problem hiding this comment.
I like how this is more explicit - everything that happens for the install happens here.
Is there a "recommended pattern"?
There was a problem hiding this comment.
I don't know if it's written down, but I think it's recommended to not override the phases directly, but rather add stuff to pre/post. It's easy to forget to invoke the hooks. (As evidenced by this expression that you're fixing now.)
There was a problem hiding this comment.
The thing is, this is the installation phase. From the perspective of "least surprising behaviour" I would assume installation is done in the installPhase. If we do this in postInstall instead, somebody cannot override that phase without not having anything installed.
I fully understand where you're coming from though - when people start messing with phases directly there is a high chance of someone breaking things or accidentally leaving out phases that should be run. But in this case this is actually doing the right thing (IMHO).
rycee
reviewed
Mar 15, 2018
| type = str; | ||
| description = '' | ||
| The interval at which to run the check and update. | ||
| See <command>man 7 systemd.time</command> for the format. |
There was a problem hiding this comment.
Instead of
See <command>man 7 systemd.time</command> for the format.it is nicer to directly reference the document:
See
<citerefentry>
<refentrytitle>systemd.time</refentrytitle>
<manvolnum>7</manvolnum>
</citerefentry>
for the format.
a) Some providers can update multiple domains - support that. b) Make "zone" and "script" configurable. Some providers require these. c) Instead of leaving the ddclient daemon running all the time, use a systemd timer to kick it off. d) Don't use a predefined user - run everything via DynamicUser e) Add documentation
|
Been running with this for a month or so. |
peterhoeg
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Some providers can handle multiple domains so add support for that.
Additionally, ddclient will by default run all the time and then do its thing at an interval. The interval is quite short by default (10 minutes) but if someone was increase that to half a day (as an example) it would be pointless to leave the ddclient running for the entire time. Instead we just let systemd handle the timing.
Running with this set-up here - so far it looks good.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)