Skip to content
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

fix: improved error messages when deps are missing #484

Merged
merged 4 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/hello-world/services/hello-container/garden.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ module:
env:
FUNCTION_ENDPOINT: ${services.hello-function.outputs.endpoint}
dependencies:
- hello-function
- hello-function
52 changes: 14 additions & 38 deletions garden-service/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions garden-service/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ import {
} from "./config/base"
import { BaseTask } from "./tasks/base"
import { LocalConfigStore } from "./config-store"
import { detectCircularDependencies } from "./util/detectCycles"
import { validateDependencies } from "./util/validate-dependencies"
import {
getLinkedSources,
ExternalSourceType,
Expand Down Expand Up @@ -837,14 +837,17 @@ export class Garden {
const moduleConfigContext = new ModuleConfigContext(
this, this.log, this.environment, Object.values(this.moduleConfigs),
)
this.moduleConfigs = await resolveTemplateStrings(this.moduleConfigs, moduleConfigContext)

await this.detectCircularDependencies()
this.moduleConfigs = await resolveTemplateStrings(this.moduleConfigs, moduleConfigContext)
this.validateDependencies()
})
}

private async detectCircularDependencies() {
return detectCircularDependencies(Object.values(this.moduleConfigs))
private validateDependencies() {
validateDependencies(
Object.values(this.moduleConfigs),
Object.keys(this.serviceNameIndex),
Object.keys(this.taskNameIndex))
}

/*
Expand Down Expand Up @@ -937,7 +940,7 @@ export class Garden {

if (this.modulesScanned) {
// need to re-run this if adding modules after initial scan
await this.detectCircularDependencies()
await this.validateDependencies()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,115 @@
*/

import dedent = require("dedent")
import { merge } from "lodash"
import * as indentString from "indent-string"
import { get, isEqual, join, set, uniqWith } from "lodash"
import { getModuleKey } from "../types/module"
import { ConfigurationError } from "../exceptions"
import { ServiceConfig } from "../config/service"
import { TaskConfig } from "../config/task"
import { ModuleConfig } from "../config/module"
import { deline } from "./string"

export type Cycle = string[]
export function validateDependencies(
moduleConfigs: ModuleConfig[], serviceNames: string[], taskNames: string[],
): void {

/*
Implements a variation on the Floyd-Warshall algorithm to compute minimal cycles.
const missingDepsError = detectMissingDependencies(moduleConfigs, serviceNames, taskNames)
const circularDepsError = detectCircularDependencies(moduleConfigs)

let errMsg = ""
let detail = {}

if (missingDepsError) {
errMsg += missingDepsError.message
detail = merge(detail, missingDepsError.detail)
}

if (circularDepsError) {
errMsg += "\n" + circularDepsError.message
detail = merge(detail, circularDepsError.detail)
}

if (missingDepsError || circularDepsError) {
throw new ConfigurationError(errMsg, detail)
}

This is approximately O(m^3) + O(s^3), where m is the number of modules and s is the number of services.
}

/**
* Looks for dependencies on non-existent modules, services or tasks, and returns an error
* if any were found.
*/
export function detectMissingDependencies(
moduleConfigs: ModuleConfig[], serviceNames: string[], taskNames: string[],
): ConfigurationError | null {

const moduleNames: Set<string> = new Set(moduleConfigs.map(m => m.name))
const runtimeNames: Set<string> = new Set([...serviceNames, ...taskNames])
const missingDepDescriptions: string[] = []

const runtimeDepTypes = [
["serviceConfigs", "Service"],
["taskConfigs", "Task"],
["testConfigs", "Test"],
]

for (const m of moduleConfigs) {

const buildDepKeys = m.build.dependencies.map(d => getModuleKey(d.name, d.plugin))

for (const missingModule of buildDepKeys.filter(k => !moduleNames.has(k))) {
missingDepDescriptions.push(
`Module '${m.name}': Unknown module '${missingModule}' referenced in build dependencies.`,
)
}

Throws an error if cycles were found.
*/
export async function detectCircularDependencies(moduleConfigs: ModuleConfig[]) {
for (const [configKey, entityName] of runtimeDepTypes) {
for (const config of m[configKey]) {
for (const missingRuntimeDep of config.dependencies.filter(d => !runtimeNames.has(d))) {
missingDepDescriptions.push(deline`
${entityName} '${config.name}' (in module '${m.name}'): Unknown service or task '${missingRuntimeDep}'
referenced in dependencies.`,
)
}
}
}

}

if (missingDepDescriptions.length > 0) {
const errMsg = "Unknown dependencies detected.\n\n" +
indentString(missingDepDescriptions.join("\n\n"), 2) + "\n"

return new ConfigurationError(errMsg, { "unknown-dependencies": missingDepDescriptions })
} else {
return null
}

}

export type Cycle = string[]

/**
* Implements a variation on the Floyd-Warshall algorithm to compute minimal cycles.
*
* This is approximately O(m^3) + O(s^3), where m is the number of modules and s is the number of services.
*
* Returns an error if cycles were found.
*/
export function detectCircularDependencies(moduleConfigs: ModuleConfig[]): ConfigurationError | null {
// Sparse matrices
const buildGraph = {}
const runtimeGraph = {}
const services: ServiceConfig[] = []
const tasks: TaskConfig[] = []

/*
There's no need to account for test dependencies here, since any circularities there
are accounted for via service dependencies.
*/
/**
* Since dependencies listed in test configs cannot introduce circularities (because
* builds/deployments/tasks/tests cannot currently depend on tests), we don't need to
* account for test dependencies here.
*/
thsig marked this conversation as resolved.
Show resolved Hide resolved
for (const module of moduleConfigs) {
// Build dependencies
for (const buildDep of module.build.dependencies) {
Expand Down Expand Up @@ -83,8 +165,10 @@ export async function detectCircularDependencies(moduleConfigs: ModuleConfig[])
detail["circular-service-or-task-dependencies"] = runtimeCyclesDescription
}

throw new ConfigurationError(errMsg, detail)
return new ConfigurationError(errMsg, detail)
}

return null
}

export function detectCycles(graph, vertices: string[]): Cycle[] {
Expand Down
Loading