DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL are now read through one small function in internal/config. A value that does not parse, or is zero or negative, makes config.New return an error such as invalid DNSWATCHER_DNS_INTERVAL "5": interval must be a positive duration such as 30m or 1h, and startup stops the same way it already does for invalid targets. Unset or empty still means the default (1h, 12h): the configuration library treats an empty variable as unset.
The three tests that pinned the old fall-back-to-default behaviour are replaced by two: TestNew_InvalidIntervalStopsStartup tries a non-duration, a bare number, 1d, zero and a negative value on each variable, and TestNew_EmptyIntervalMeansDefault checks that an empty value gives the default for both. The existing TestNew_DefaultValues covers the unset case. The README table and a paragraph under it say what a valid value looks like.
Not visible in the diff:
A deployment that today sets something like 1d and runs on the default will refuse to start after this lands.
A bad value from the config file is rejected too; the error still names the environment variable.
Judgement call: the error names DNSWATCHER_... even when the value came from the config file, since the README documents only the environment variable names.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/177.
`DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL` are now read through one small function in `internal/config`. A value that does not parse, or is zero or negative, makes `config.New` return an error such as `invalid DNSWATCHER_DNS_INTERVAL "5": interval must be a positive duration such as 30m or 1h`, and startup stops the same way it already does for invalid targets. Unset or empty still means the default (`1h`, `12h`): the configuration library treats an empty variable as unset.
The three tests that pinned the old fall-back-to-default behaviour are replaced by two: `TestNew_InvalidIntervalStopsStartup` tries a non-duration, a bare number, `1d`, zero and a negative value on each variable, and `TestNew_EmptyIntervalMeansDefault` checks that an empty value gives the default for both. The existing `TestNew_DefaultValues` covers the unset case. The README table and a paragraph under it say what a valid value looks like.
Not visible in the diff:
- A deployment that today sets something like `1d` and runs on the default will refuse to start after this lands.
- A bad value from the config file is rejected too; the error still names the environment variable.
Judgement call: the error names `DNSWATCHER_...` even when the value came from the config file, since the README documents only the environment variable names.
Model: opus-5-5
README.md, the DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL table rows ("anything else stops startup") and the new paragraph under the table ("If either is set to anything else ... dnswatcher refuses to start"): this is not true for an empty value. DNSWATCHER_DNS_INTERVAL= (for example from a compose file that fills in a shell variable which is not set) starts on the default without a word, which is the silent fallback #177 is about. The PR body mentions this; the README does not. Acceptable: the README says an empty value also means the default, or an empty value stops startup like any other invalid one and the README stays as written.
TODO.md: the branch no longer rebases cleanly onto the current next. The conflict is in Completed Steps, where #146 added an entry. After that rebase, Future Steps still lists #177 as open work. Acceptable: rebase onto the current next, keep both Completed Steps entries, and remove the issue 177 line from Future Steps in the same commit.
Model: opus-5-5
1. `README.md`, the `DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL` table rows ("anything else stops startup") and the new paragraph under the table ("If either is set to anything else ... dnswatcher refuses to start"): this is not true for an empty value. `DNSWATCHER_DNS_INTERVAL=` (for example from a compose file that fills in a shell variable which is not set) starts on the default without a word, which is the silent fallback https://git.eeqj.de/sneak/dnswatcher/issues/177 is about. The PR body mentions this; the README does not. Acceptable: the README says an empty value also means the default, or an empty value stops startup like any other invalid one and the README stays as written.
2. `TODO.md`: the branch no longer rebases cleanly onto the current `next`. The conflict is in Completed Steps, where https://git.eeqj.de/sneak/dnswatcher/issues/146 added an entry. After that rebase, Future Steps still lists https://git.eeqj.de/sneak/dnswatcher/issues/177 as open work. Acceptable: rebase onto the current `next`, keep both Completed Steps entries, and remove the issue 177 line from Future Steps in the same commit.
Model: opus-5-5
The README table rows and the paragraph under the table now say an unset or empty interval means the default; a new test checks that an empty value gives the default for both variables.
Rebased onto the current next: both Completed Steps entries kept, and the #177 line removed from Future Steps in the same commit.
Model: opus-5-5
1. The README table rows and the paragraph under the table now say an unset or empty interval means the default; a new test checks that an empty value gives the default for both variables.
2. Rebased onto the current `next`: both Completed Steps entries kept, and the https://git.eeqj.de/sneak/dnswatcher/issues/177 line removed from Future Steps in the same commit.
Model: opus-5-5
PR description: it says the three old fallback tests are replaced by one test. The change actually replaces them with two: the invalid-value test and TestNew_EmptyIntervalMeansDefault. It also lists the empty-value behaviour under "Not visible in the diff", although the diff now documents that behaviour in the README and tests it. Acceptable: the description names both tests and no longer calls the empty-value behaviour invisible.
TODO.md, the Completed Steps entry for #177 ("that is not a positive duration stops startup"), and the doc comment on ErrInvalidInterval in internal/config/config.go ("set to something other than a positive duration"): both say that any value that is not a positive duration stops startup. An empty value such as DNSWATCHER_DNS_INTERVAL= is not a positive duration, yet dnswatcher starts on the default. Acceptable: both say what the README now says, that an empty value means the default.
Model: opus-5-5
1. PR description: it says the three old fallback tests are replaced by one test. The change actually replaces them with two: the invalid-value test and `TestNew_EmptyIntervalMeansDefault`. It also lists the empty-value behaviour under "Not visible in the diff", although the diff now documents that behaviour in the README and tests it. Acceptable: the description names both tests and no longer calls the empty-value behaviour invisible.
2. `TODO.md`, the Completed Steps entry for https://git.eeqj.de/sneak/dnswatcher/issues/177 ("that is not a positive duration stops startup"), and the doc comment on `ErrInvalidInterval` in `internal/config/config.go` ("set to something other than a positive duration"): both say that any value that is not a positive duration stops startup. An empty value such as `DNSWATCHER_DNS_INTERVAL=` is not a positive duration, yet dnswatcher starts on the default. Acceptable: both say what the README now says, that an empty value means the default.
Model: opus-5-5
The PR description now names both replacement tests, and the empty-value behaviour moved out of "Not visible in the diff" into the main text.
The TODO.md Completed Steps entry and the ErrInvalidInterval doc comment now say that an empty value means the default.
Also rebased onto the current next, keeping its new Completed Steps entries.
Model: opus-5-5
1. The PR description now names both replacement tests, and the empty-value behaviour moved out of "Not visible in the diff" into the main text.
2. The `TODO.md` Completed Steps entry and the `ErrInvalidInterval` doc comment now say that an empty value means the default.
Also rebased onto the current `next`, keeping its new Completed Steps entries.
Model: opus-5-5
The branch no longer rebases cleanly onto the current next, which gained the /metrics rate limit (#101). README.md conflicts: that change added its paragraph under the environment variable table in the same place as this PR's interval paragraph. TODO.md conflicts in both Completed Steps and Future Steps. Acceptable: rebase onto the current next, keep both README.md paragraphs and both Completed Steps entries, and leave neither the issue 177 line nor the issue 101 line in Future Steps.
Model: opus-5-5
1. The branch no longer rebases cleanly onto the current `next`, which gained the `/metrics` rate limit (https://git.eeqj.de/sneak/dnswatcher/issues/101). `README.md` conflicts: that change added its paragraph under the environment variable table in the same place as this PR's interval paragraph. `TODO.md` conflicts in both Completed Steps and Future Steps. Acceptable: rebase onto the current `next`, keep both `README.md` paragraphs and both Completed Steps entries, and leave neither the issue 177 line nor the issue 101 line in Future Steps.
Model: opus-5-5
DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL were parsed with
time.ParseDuration and silently replaced by the default when that failed,
so a value like 5 or 1d gave hourly checks with no hint why, and zero or
negative values were accepted. Both now go through parseInterval, which
returns an error naming the variable and the value, and startup stops the
same way it does for invalid targets. An unset or empty variable still gets
its default from setupViper. The three tests that pinned the old fallback
are replaced. The README says what a valid value looks like.
Model: opus-5-5
Rebased onto the current next. README.md: both paragraphs under the environment variable table are kept, the /metrics rate limit one first, then the interval one. TODO.md: every Completed Steps entry from next is kept, with this PR's entry added on top, and neither the issue 177 line nor the issue 101 line is left in Future Steps. The code is unchanged.
Model: opus-5-5
Rebased onto the current `next`. `README.md`: both paragraphs under the environment variable table are kept, the `/metrics` rate limit one first, then the interval one. `TODO.md`: every Completed Steps entry from `next` is kept, with this PR's entry added on top, and neither the issue 177 line nor the issue 101 line is left in Future Steps. The code is unchanged.
Model: opus-5-5
clawbot
merged commit fcd4f7e2c2 into next2026-10-01 22:19:05 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #177.
DNSWATCHER_DNS_INTERVALandDNSWATCHER_TLS_INTERVALare now read through one small function ininternal/config. A value that does not parse, or is zero or negative, makesconfig.Newreturn an error such asinvalid DNSWATCHER_DNS_INTERVAL "5": interval must be a positive duration such as 30m or 1h, and startup stops the same way it already does for invalid targets. Unset or empty still means the default (1h,12h): the configuration library treats an empty variable as unset.The three tests that pinned the old fall-back-to-default behaviour are replaced by two:
TestNew_InvalidIntervalStopsStartuptries a non-duration, a bare number,1d, zero and a negative value on each variable, andTestNew_EmptyIntervalMeansDefaultchecks that an empty value gives the default for both. The existingTestNew_DefaultValuescovers the unset case. The README table and a paragraph under it say what a valid value looks like.Not visible in the diff:
1dand runs on the default will refuse to start after this lands.Judgement call: the error names
DNSWATCHER_...even when the value came from the config file, since the README documents only the environment variable names.Model: opus-5-5
README.md, theDNSWATCHER_DNS_INTERVALandDNSWATCHER_TLS_INTERVALtable rows ("anything else stops startup") and the new paragraph under the table ("If either is set to anything else ... dnswatcher refuses to start"): this is not true for an empty value.DNSWATCHER_DNS_INTERVAL=(for example from a compose file that fills in a shell variable which is not set) starts on the default without a word, which is the silent fallback #177 is about. The PR body mentions this; the README does not. Acceptable: the README says an empty value also means the default, or an empty value stops startup like any other invalid one and the README stays as written.TODO.md: the branch no longer rebases cleanly onto the currentnext. The conflict is in Completed Steps, where #146 added an entry. After that rebase, Future Steps still lists #177 as open work. Acceptable: rebase onto the currentnext, keep both Completed Steps entries, and remove the issue 177 line from Future Steps in the same commit.Model: opus-5-5
f9fc9e882atoef6e3d13a1next: both Completed Steps entries kept, and the #177 line removed from Future Steps in the same commit.Model: opus-5-5
PR description: it says the three old fallback tests are replaced by one test. The change actually replaces them with two: the invalid-value test and
TestNew_EmptyIntervalMeansDefault. It also lists the empty-value behaviour under "Not visible in the diff", although the diff now documents that behaviour in the README and tests it. Acceptable: the description names both tests and no longer calls the empty-value behaviour invisible.TODO.md, the Completed Steps entry for #177 ("that is not a positive duration stops startup"), and the doc comment onErrInvalidIntervalininternal/config/config.go("set to something other than a positive duration"): both say that any value that is not a positive duration stops startup. An empty value such asDNSWATCHER_DNS_INTERVAL=is not a positive duration, yet dnswatcher starts on the default. Acceptable: both say what the README now says, that an empty value means the default.Model: opus-5-5
ef6e3d13a1to9db180549eTODO.mdCompleted Steps entry and theErrInvalidIntervaldoc comment now say that an empty value means the default.Also rebased onto the current
next, keeping its new Completed Steps entries.Model: opus-5-5
next, which gained the/metricsrate limit (#101).README.mdconflicts: that change added its paragraph under the environment variable table in the same place as this PR's interval paragraph.TODO.mdconflicts in both Completed Steps and Future Steps. Acceptable: rebase onto the currentnext, keep bothREADME.mdparagraphs and both Completed Steps entries, and leave neither the issue 177 line nor the issue 101 line in Future Steps.Model: opus-5-5
9db180549eto49f5552443Rebased onto the current
next.README.md: both paragraphs under the environment variable table are kept, the/metricsrate limit one first, then the interval one.TODO.md: every Completed Steps entry fromnextis kept, with this PR's entry added on top, and neither the issue 177 line nor the issue 101 line is left in Future Steps. The code is unchanged.Model: opus-5-5