Ignore a leftover builds.maxConcurrent instead of refusing to start

The concurrency limit moved to executor.maxConcurrent without an alias, so
the old key binds a scalar where a build definition belongs and failed the
whole configuration. But that configuration is committed in the watched
repository, and a repository's master is not always changeable right away —
an installation must not be stuck on a key it is meant to forget.

A `builds` entry that is not a mapping is now dropped with a warning (once
per key, the config is loaded every poll cycle), naming executor.maxConcurrent
for the key that moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
mhoennig
2026-08-29 07:37:10 +02:00
co-authored by Claude Opus 5
parent f5871a0442
commit ca3e758cdc
4 changed files with 57 additions and 4 deletions
+2 -1
View File
@@ -287,7 +287,8 @@ The builds still run in their branch's worktree, one build per branch at a time,
`branches.<name>.autoBuild` (`enabled` + `times`) is the deprecated pre-ADR-0007 schedule, kept for compatibility: it rebuilds the branch's own pool with its regular command and logs a deprecation warning. `branches.<name>.autoBuild` (`enabled` + `times`) is the deprecated pre-ADR-0007 schedule, kept for compatibility: it rebuilds the branch's own pool with its regular command and logs a deprecation warning.
`autoBuild.times` entries carrying their own `buildCommand`/`name` (a short-lived v0.9.13 syntax) are no longer supported — use a build definition. `autoBuild.times` entries carrying their own `buildCommand`/`name` (a short-lived v0.9.13 syntax) are no longer supported — use a build definition.
The concurrency limit that used to live in this section moved to `executor.maxConcurrent` without an alias — a leftover `builds.maxConcurrent` key is rejected as an invalid build definition. The concurrency limit that used to live in this section moved to `executor.maxConcurrent` without an alias.
A leftover `builds.maxConcurrent` key (or any other scalar where a definition belongs) is ignored with a warning, not a startup failure — a committed config cannot always be changed right away.
### Notes on `watcher.fastForwardLocalRefs` ### Notes on `watcher.fastForwardLocalRefs`
@@ -6,19 +6,26 @@ import com.fasterxml.jackson.databind.SerializationFeature
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory import com.fasterxml.jackson.dataformat.yaml.YAMLFactory
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator
import com.fasterxml.jackson.module.kotlin.registerKotlinModule import com.fasterxml.jackson.module.kotlin.registerKotlinModule
import org.slf4j.LoggerFactory
import org.springframework.stereotype.Service import org.springframework.stereotype.Service
import java.io.File import java.io.File
import java.nio.file.Path import java.nio.file.Path
import java.nio.file.Paths import java.nio.file.Paths
import java.util.concurrent.ConcurrentHashMap
@Service @Service
class ConfigLoader { class ConfigLoader {
private val log = LoggerFactory.getLogger(ConfigLoader::class.java)
private val yaml = private val yaml =
ObjectMapper(YAMLFactory().disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER)) ObjectMapper(YAMLFactory().disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER))
.registerKotlinModule() .registerKotlinModule()
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false) .configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false)
/** Keys already reported by [dropNonDefinitionBuilds]; the config is loaded on every poll cycle. */
private val warnedBuildKeys = ConcurrentHashMap.newKeySet<String>()
fun load(workingDir: Path = Paths.get(".")): GitTallyConfig = toConfig(loadRaw(workingDir)) fun load(workingDir: Path = Paths.get(".")): GitTallyConfig = toConfig(loadRaw(workingDir))
/** /**
@@ -59,11 +66,38 @@ class ConfigLoader {
if (raw.isEmpty()) { if (raw.isEmpty()) {
GitTallyConfig() GitTallyConfig()
} else { } else {
yaml.convertValue(mergeBranchDefaults(raw), GitTallyConfig::class.java) yaml.convertValue(mergeBranchDefaults(dropNonDefinitionBuilds(raw)), GitTallyConfig::class.java)
} }
return defaultPublicBaseUrl(config) return defaultPublicBaseUrl(config)
} }
/**
* Ignores `builds` entries that are not a build definition — a scalar where a
* definition belongs, most likely the `builds.maxConcurrent` key that moved to
* `executor.maxConcurrent`. Such a leftover is a warning, not a startup failure:
* the config lives in a repository whose `master` may not be changeable right now,
* and the rest of it is perfectly usable.
*/
@Suppress("UNCHECKED_CAST")
private fun dropNonDefinitionBuilds(raw: Map<String, Any?>): Map<String, Any?> {
val builds = raw["builds"] as? Map<String, Any?> ?: return raw
val definitions = builds.filterValues { it is Map<*, *> }
if (definitions.size == builds.size) {
return raw
}
for (key in builds.keys - definitions.keys) {
if (!warnedBuildKeys.add(key)) {
continue
}
if (key == "maxConcurrent") {
log.warn("ignoring builds.maxConcurrent; the concurrency limit is executor.maxConcurrent since v0.9.15")
} else {
log.warn("ignoring builds.{}: a build definition must be a mapping of keys", key)
}
}
return raw + ("builds" to definitions)
}
/** /**
* Removes the keys a branch must never override: the secret and host-side top-level * Removes the keys a branch must never override: the secret and host-side top-level
* sections, the per-branch trust gate, and the docker sandbox policy. * sections, the per-branch trust gate, and the docker sandbox policy.
+3 -2
View File
@@ -23,8 +23,9 @@
<li><strong>Changed:</strong> the build concurrency limit moved from <li><strong>Changed:</strong> the build concurrency limit moved from
<code>builds.maxConcurrent</code> to <code>executor.maxConcurrent</code> (default 1, <code>builds.maxConcurrent</code> to <code>executor.maxConcurrent</code> (default 1,
no compatibility alias) — the <code>builds</code> section now holds build definitions no compatibility alias) — the <code>builds</code> section now holds build definitions
only. A leftover <code>builds.maxConcurrent</code> key is rejected as an invalid only. A leftover <code>builds.maxConcurrent</code> key is ignored with a warning
build definition.</li> instead of failing the configuration, so an installation keeps running until its
committed config can be updated.</li>
</ul> </ul>
<h2>v0.9.14 <span class="muted">— 2026-08-28</span></h2> <h2>v0.9.14 <span class="muted">— 2026-08-28</span></h2>
@@ -92,6 +92,23 @@ class ConfigLoaderTest : FunSpec() {
loader.load(dir).effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(onPush = false) loader.load(dir).effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(onPush = false)
} }
test("a leftover builds.maxConcurrent is ignored instead of failing the config") {
val dir = Files.createTempDirectory("gittally-test")
dir.resolve(".gittally.yml").toFile().writeText(
"""
builds:
maxConcurrent: 1
pitest:
buildCommand: ./gradlew piTestFull
""".trimIndent(),
)
val config = loader.load(dir)
config.executor.maxConcurrent shouldBe 1
config.buildDefinitions.keys shouldBe setOf("pitest")
}
test("a branch may redefine the builds section for its own builds") { test("a branch may redefine the builds section for its own builds") {
val dir = Files.createTempDirectory("gittally-test") val dir = Files.createTempDirectory("gittally-test")
dir.resolve(".gittally.yml").toFile().writeText( dir.resolve(".gittally.yml").toFile().writeText(