Repository navigation
Skip an optional dependency missing from the configuration (0.3.1) - #2
Merged
Merged
Conversation
topologicalSort threw "depends on X which doesn't exist" for every declared dependency, optional ones included, so startSystem, haltSystem and reconcileSystem could not run a config that leaves an optional dependency out, nor reconcile to one. dependencyEdges (dependencies.ts) names the dependencies that are edges in a system: every required one, and the optional ones it has. The sort, the topology and reconcile's dependents walk it; a missing required dependency still throws, and the dependent receives undefined for the missing optional one, as for a failed one. Reconciling to a config that adds an optional dependency restarts its unchanged dependents, which the halt set did not include: the halt set is now also every restarted resource that was in the previous config. 0.3.1.
This file contains hidden or 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
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.
Problem
topologicalSortthrewResource "X" depends on "Y" which doesn't exist in the system configfor every declared dependency, optional ones included. An optional dependency exists so a resource can run without it, yet a config that leaves it out could not be started, halted, or reconciled to. In practice: resourcex:githubdeclaresoptional: ['x:reviews']; when reviews are turned off the next config has nox:reviews, andreconcileSystem(prev, system, next)threw instead of restartingx:githubwithout reviews.Once that is fixed, the opposite direction exposes a second bug. Reconciling to a config that adds an optional dependency restarted its unchanged dependents without halting their previous instances, because the halt set only covered changed and removed resources. Before, this could not happen, since a previous config with a missing dependency could never start.
Fix
dependencyEdges(deps, resources)insrc/dependencies.tsreturns the dependencies that are edges in a system: every required one, plus each optional one the system has.topologicalSort,buildTopology(and through itformatTopology,toMermaid,toDot,toJSON) and reconcile'swithDependentsall walkdependencyEdges. A missing optional dependency adds no edge. A missing required one still reaches the existing throw.startInOrderis unchanged: it already resolves an absent id toundefined, which is how a failed optional dependency is handled.reconcileSystemnow halts every restarted resource that was in the previous config, in addition to the changed and removed resources and their dependents.Tests
Before: 86 passing. After: 93 passing (7 new). Six of the new specs fail against the old source. The seventh, a missing required dependency beside a missing optional one at the
startSystemlevel, passes on both versions; it locks the existing throw in place.{ counter, absent: undefined }, and halts cleanly. A missing required dependency next to a missing optional one still throws.npm test,npm run type:checkandnpm run buildall pass.