Skip to content

Commit

Permalink
improvement(core): warn on large file count in modules
Browse files Browse the repository at this point in the history
This includes a new command and mechanism to suppress individual
warnings. We can use that to emit more warnings of this nature, where
the warning may or may not be relevant and could be annoying to see on
every invocation.
  • Loading branch information
edvald committed Sep 8, 2020
1 parent 852b85c commit 3ee20dc
Show file tree
Hide file tree
Showing 12 changed files with 257 additions and 32 deletions.
2 changes: 1 addition & 1 deletion cli/src/add-version-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ async function addVersionFiles() {
const versionFilePath = resolve(path, GARDEN_VERSIONFILE_NAME)

const vcsHandler = new GitHandler(garden.gardenDirPath, garden.dotIgnoreFiles)
const treeVersion = await vcsHandler.getTreeVersion(garden.log, config)
const treeVersion = await vcsHandler.getTreeVersion(garden.log, garden.projectName, config)

// tslint:disable-next-line: no-console
console.log(`${config.name} -> ${relative(STATIC_DIR, versionFilePath)}`)
Expand Down
41 changes: 41 additions & 0 deletions core/src/commands/util/hide-warning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (C) 2018-2020 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { Command, CommandParams } from "../base"
import dedent from "dedent"
import { StringParameter } from "../../cli/params"
import { Warning } from "../../db/entities/warning"

const hideWarningArgs = {
key: new StringParameter({
help: "The key of the warning to hide (this will be shown along with relevant warning messages).",
required: true,
}),
}

type Args = typeof hideWarningArgs

export class HideWarningCommand extends Command<Args, {}> {
name = "hide-warning"
help = "Hide a specific warning message."
cliOnly = true

noProject = true

description = dedent`
Hides the specified warning message. The command and key is generally provided along with displayed warning messages.
`

arguments = hideWarningArgs

async action({ args }: CommandParams<Args, {}>) {
await Warning.hide(args.key)

return {}
}
}
3 changes: 2 additions & 1 deletion core/src/commands/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@

import { CommandGroup } from "../base"
import { FetchToolsCommand } from "./fetch-tools"
import { HideWarningCommand } from "./hide-warning"

export class UtilCommand extends CommandGroup {
name = "util"
help = "Misc utility commands."

subCommands = [FetchToolsCommand]
subCommands = [FetchToolsCommand, HideWarningCommand]
}
4 changes: 3 additions & 1 deletion core/src/db/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ export function getConnection(): Connection {
const { LocalAddress } = require("./entities/local-address")
const { ClientAuthToken } = require("./entities/client-auth-token")
const { GardenProcess } = require("./entities/garden-process")
const { Warning } = require("./entities/warning")

// Prepare the connection (the ormconfig.json in the static dir is only used for the typeorm CLI during dev)
const options: ConnectionOptions = {
type: "sqlite",
database: databasePath,
// IMPORTANT: All entities and migrations need to be manually referenced here because of how we
// package the garden binary
entities: [LocalAddress, ClientAuthToken, GardenProcess],
entities: [LocalAddress, ClientAuthToken, GardenProcess, Warning],
migrations: [],
// Auto-create new tables on init
synchronize: true,
Expand Down
11 changes: 6 additions & 5 deletions core/src/db/entities/local-address.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ export class LocalAddress extends GardenEntity {
}

static async resolve(values: { projectName: string; moduleName: string; serviceName: string; hostname: string }) {
await this.createQueryBuilder()
.insert()
.values({ ...values })
.onConflict("DO NOTHING")
.execute()
try {
await this.createQueryBuilder()
.insert()
.values({ ...values })
.execute()
} catch {}

return this.findOneOrFail({ where: values })
}
Expand Down
41 changes: 41 additions & 0 deletions core/src/db/entities/warning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (C) 2018-2020 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { Entity, Column, Index } from "typeorm"
import { GardenEntity } from "../base-entity"
import { LogEntry } from "../../logger/log-entry"
import chalk from "chalk"

/**
* Provides a mechanism to emit warnings that the user can then hide, via the `garden util hide-warning` command.
*/
@Entity()
@Index(["key"], { unique: true })
export class Warning extends GardenEntity {
@Column()
key: string

@Column()
hidden: boolean

static async emit({ key, log, message }: { key: string; log: LogEntry; message: string }) {
const existing = await this.findOne({ where: { key } })

if (!existing || !existing.hidden) {
log.warn(
chalk.yellow(message + `\nRun ${chalk.underline(`garden util hide-warning ${key}`)} to disable this warning.`)
)
}
}

static async hide(key: string) {
try {
await this.createQueryBuilder().insert().values({ key, hidden: true }).execute()
} catch {}
}
}
2 changes: 1 addition & 1 deletion core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ export class Garden {
const dependencies = await this.getRawModuleConfigs(dependencyKeys)
const cacheContexts = dependencies.concat([moduleConfig]).map((c) => getModuleCacheContext(c))

const version = await this.vcs.resolveVersion(this.log, moduleConfig, dependencies)
const version = await this.vcs.resolveVersion(this.log, this.projectName, moduleConfig, dependencies)

this.cache.set(cacheKey, version, ...cacheContexts)
return version
Expand Down
25 changes: 20 additions & 5 deletions core/src/vcs/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ import { ExternalSourceType, getRemoteSourcesDirname, getRemoteSourceRelPath } f
import { ModuleConfig, serializeConfig } from "../config/module"
import { LogEntry } from "../logger/log-entry"
import { treeVersionSchema, moduleVersionSchema } from "../config/common"
import { Warning } from "../db/entities/warning"
import { dedent } from "../util/string"

export const NEW_MODULE_VERSION = "0000000000"
const fileCountWarningThreshold = 10000

export interface TreeVersion {
contentHash: string
Expand Down Expand Up @@ -72,7 +75,7 @@ export abstract class VcsHandler {
abstract async updateRemoteSource(params: RemoteSourceParams): Promise<void>
abstract async getOriginName(log: LogEntry): Promise<string | undefined>

async getTreeVersion(log: LogEntry, moduleConfig: ModuleConfig): Promise<TreeVersion> {
async getTreeVersion(log: LogEntry, projectName: string, moduleConfig: ModuleConfig): Promise<TreeVersion> {
const configPath = moduleConfig.configPath

let files = await this.getFiles({
Expand All @@ -83,6 +86,17 @@ export abstract class VcsHandler {
exclude: moduleConfig.exclude,
})

if (files.length > fileCountWarningThreshold) {
await Warning.emit({
key: `${projectName}-filecount-${moduleConfig.name}`,
log,
message: dedent`
Large number of files (${files.length}) found in module ${moduleConfig.name}. You may need to configure file exclusions.
See https://docs.garden.io/using-garden/configuration-overview#including-excluding-files-and-directories for details.
`,
})
}

files = sortBy(files, "path")
// Don't include the config file in the file list
.filter((f) => !configPath || f.path !== configPath)
Expand All @@ -92,19 +106,20 @@ export abstract class VcsHandler {
return { contentHash, files: files.map((f) => f.path) }
}

async resolveTreeVersion(log: LogEntry, moduleConfig: ModuleConfig): Promise<TreeVersion> {
async resolveTreeVersion(log: LogEntry, projectName: string, moduleConfig: ModuleConfig): Promise<TreeVersion> {
// the version file is used internally to specify versions outside of source control
const versionFilePath = join(moduleConfig.path, GARDEN_TREEVERSION_FILENAME)
const fileVersion = await readTreeVersionFile(versionFilePath)
return fileVersion || (await this.getTreeVersion(log, moduleConfig))
return fileVersion || (await this.getTreeVersion(log, projectName, moduleConfig))
}

async resolveVersion(
log: LogEntry,
projectName: string,
moduleConfig: ModuleConfig,
dependencies: ModuleConfig[]
): Promise<ModuleVersion> {
const treeVersion = await this.resolveTreeVersion(log, moduleConfig)
const treeVersion = await this.resolveTreeVersion(log, projectName, moduleConfig)

validateSchema(treeVersion, treeVersionSchema(), {
context: `${this.name} tree version for module at ${moduleConfig.path}`,
Expand All @@ -121,7 +136,7 @@ export abstract class VcsHandler {

const namedDependencyVersions = await Bluebird.map(dependencies, async (m: ModuleConfig) => ({
name: m.name,
...(await this.resolveTreeVersion(log, m)),
...(await this.resolveTreeVersion(log, projectName, m)),
}))
const dependencyVersions = mapValues(keyBy(namedDependencyVersions, "name"), (v) => omit(v, "name"))

Expand Down
43 changes: 43 additions & 0 deletions core/test/unit/src/commands/util/hide-warning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (C) 2018-2020 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { withDefaultGlobalOpts, getLogMessages, projectRootA } from "../../../../helpers"
import { expect } from "chai"
import { Warning } from "../../../../../src/db/entities/warning"
import { getConnection } from "../../../../../src/db/connection"
import { HideWarningCommand } from "../../../../../src/commands/util/hide-warning"
import { makeDummyGarden } from "../../../../../src/cli/cli"
import { randomString } from "../../../../../src/util/string"

describe("HideWarningCommand", () => {
it("should hide a warning message", async () => {
const garden = await makeDummyGarden(projectRootA)
const log = garden.log.placeholder()
const cmd = new HideWarningCommand()
const key = randomString(10)

try {
await cmd.action({
garden,
args: { key },
opts: withDefaultGlobalOpts({}),
log: garden.log,
headerLog: garden.log,
footerLog: garden.log,
})
await Warning.emit({
key,
log,
message: "foo",
})
expect(getLogMessages(log).length).to.equal(0)
} finally {
await getConnection().getRepository(Warning).createQueryBuilder().delete().where({ key }).execute()
}
})
})
59 changes: 59 additions & 0 deletions core/test/unit/src/db/entities/warning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright (C) 2018-2020 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { expect } from "chai"
import { randomString } from "../../../../../src/util/string"
import { ensureConnected, getConnection } from "../../../../../src/db/connection"
import { Warning } from "../../../../../src/db/entities/warning"
import { getLogger } from "../../../../../src/logger/logger"
import { getLogMessages } from "../../../../helpers"

describe("Warning", () => {
const key = randomString(10)

before(async () => {
await ensureConnected()
})

afterEach(async () => {
await getConnection().getRepository(Warning).createQueryBuilder().delete().where({ key }).execute()
})

describe("hide", () => {
it("should flag a warning key as hidden", async () => {
await Warning.hide(key)
const record = await Warning.findOneOrFail({ where: { key } })
expect(record.hidden).to.be.true
})

it("should be a no-op if a key is already hidden", async () => {
await Warning.hide(key)
await Warning.hide(key)
})
})

describe("emit", () => {
it("should log a warning if the key has not been hidden", async () => {
const log = getLogger().placeholder()
const message = "Oh noes!"
await Warning.emit({ key, log, message })
const logs = getLogMessages(log)
expect(logs.length).to.equal(1)
expect(logs[0]).to.equal(message + `\nRun garden util hide-warning ${key} to disable this warning.`)
})

it("should not log a warning if the key has been hidden", async () => {
const log = getLogger().placeholder()
const message = "Oh noes!"
await Warning.hide(key)
await Warning.emit({ key, log, message })
const logs = getLogMessages(log)
expect(logs.length).to.equal(0)
})
})
})
Loading

0 comments on commit 3ee20dc

Please sign in to comment.