Skip to content

Commit

Permalink
fix: don't use module exclude field for optimization + add test (TBS)
Browse files Browse the repository at this point in the history
Turns out using module exclude fields would need more elaborate logic,
in the case of overlapping module sources. So I removed that
optimization for now.
  • Loading branch information
edvald committed Nov 8, 2019
1 parent 95cbd21 commit 6d30b7c
Show file tree
Hide file tree
Showing 14 changed files with 71 additions and 15 deletions.
4 changes: 2 additions & 2 deletions docs/guides/configuration-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ Here we only scan the `modules` directory, but exclude the `modules/tmp` directo

If you specify a list with `include`, only those patterns are included. If you then specify one or more `exclude` patterns, those are filtered out of the ones matched by `include`. If you _only_ specify `exclude`, those patterns will be filtered out of all paths in the project directory.

The `modules.exclude` field is also used to limit the number of files and directories Garden watches for changes while running. Use that if you have a large number of files/directories in your project that you do not need to watch, or if you are seeing excessive CPU/RAM usage.
The `modules.exclude` field is also used to limit the number of files and directories Garden watches for changes while running. Use that if you have a large number of files/directories in your project that you do not need to watch, or if you are seeing excessive CPU/RAM usage. The `modules.include` field has no effect on which paths Garden watches for changes.

#### Module include/exclude

Expand All @@ -377,7 +377,7 @@ Here we only include the `Dockerfile` and all the `.py` files under `my-sources/

If you specify a list with `include`, only those files/patterns are included. If you then specify one or more `exclude` files or patterns, those are filtered out of the files matched by `include`. If you _only_ specify `exclude`, those patterns will be filtered out of all files in the module directory.

The `exclude` field on modules is also used to limit the number of files and directories Garden watches for changes while running. Use that if you have a large number of files/directories in your module that you do not need to watch, or if you are seeing excessive CPU/RAM usage.
Note that the module `include` and `exclude` fields have no effect on which paths Garden watches for changes. Use the project `modules.exclude` field (described above) for that purpose.

#### .ignore files

Expand Down
9 changes: 9 additions & 0 deletions docs/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ Note that you can also _exclude_ path using the `exclude` field or by placing `.
source tree, which use the same format as `.gitignore` files. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories) for details.

Unlike the `exclude` field, the paths/globs specified here have _no effect_ on which files and directories
Garden watches for changes. Use the `exclude` field to affect those, if you have large directories that
should not be watched for changes.

Also note that specifying an empty list here means _no paths_ should be included.

| Type | Required |
Expand All @@ -198,6 +202,7 @@ modules:
[modules](#modules) > exclude

Specify a list of POSIX-style paths or glob patterns that should be excluded when scanning for modules.
The filters here also affect which files and directories are watched for changes. So if you have a large number of directories in your project that should not be watched, you should specify them here. The `include` field does _not_ affect which files are watched.
Note that you can also explicitly _include_ files using the `include` field. If you also specify the `include` field, the paths/patterns specified here are filtered from the files matched by `include`. See the [Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories) for details.

| Type | Required |
Expand Down Expand Up @@ -527,6 +532,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/container.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/exec.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/kubernetes.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/maven-container.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/openfaas.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/module-types/terraform.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ Note that you can also explicitly _include_ files using the `include` field. If
`include` field, the files/patterns specified here are filtered from the files matched by `include`. See the
[Configuration Files guide](https://docs.garden.io/guides/configuration-files#including-excluding-files-and-directories)for details.

Unlike the `modules.exclude` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project `modules.exclude` field to affect those, if you have
large directories that should not be watched for changes.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
4 changes: 2 additions & 2 deletions garden-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@
"e2e": "cd test/e2e && ../../bin/garden test",
"e2e-project": "node build/test/e2e/e2e-project.js",
"pkg": "./bin/build-pkg.sh",
"test": "node_modules/.bin/mocha",
"test": "node_modules/.bin/mocha --opts test/mocha.opts",
"tsc": "tsc -p . --outDir build --declaration --declarationMap --sourceMap",
"view-report": "open coverage/index.html",
"watch": "npm run tsc -- -w"
Expand All @@ -230,4 +230,4 @@
]
},
"gitHead": "b0647221a4d2ff06952bae58000b104215aed922"
}
}
7 changes: 6 additions & 1 deletion garden-service/src/config/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ export const baseModuleSpecSchema = joi
Note that you can also explicitly _include_ files using the \`include\` field. If you also specify the
\`include\` field, the files/patterns specified here are filtered from the files matched by \`include\`. See the
[Configuration Files guide](${includeGuideLink})for details.`
[Configuration Files guide](${includeGuideLink})for details.
Unlike the \`modules.exclude\` field in the project config, the filters here have _no effect_ on which files
and directories are watched for changes. Use the project \`modules.exclude\` field to affect those, if you have
large directories that should not be watched for changes.
`
)
.example([["tmp/**/*", "*.log"], {}]),
repositoryUrl: joiRepositoryUrl().description(
Expand Down
17 changes: 14 additions & 3 deletions garden-service/src/config/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,24 +166,35 @@ const projectModulesSchema = joi.object().keys({
.array()
.items(joi.string().posixPath({ subPathOnly: true }))
.description(
dedent`Specify a list of POSIX-style paths or globs that should be scanned for Garden modules.
dedent`
Specify a list of POSIX-style paths or globs that should be scanned for Garden modules.
Note that you can also _exclude_ path using the \`exclude\` field or by placing \`.gardenignore\` files in your
source tree, which use the same format as \`.gitignore\` files. See the
[Configuration Files guide](${includeGuideLink}) for details.
Unlike the \`exclude\` field, the paths/globs specified here have _no effect_ on which files and directories
Garden watches for changes. Use the \`exclude\` field to affect those, if you have large directories that
should not be watched for changes.
Also note that specifying an empty list here means _no paths_ should be included.`
)
.example([["modules/**/*"], {}]),
exclude: joi
.array()
.items(joi.string().posixPath({ subPathOnly: true }))
.description(
deline`Specify a list of POSIX-style paths or glob patterns that should be excluded when scanning for modules.
deline`
Specify a list of POSIX-style paths or glob patterns that should be excluded when scanning for modules.
The filters here also affect which files and directories are watched for changes. So if you have a large number
of directories in your project that should not be watched, you should specify them here. The \`include\` field
does _not_ affect which files are watched.
Note that you can also explicitly _include_ files using the \`include\` field. If you also specify the
\`include\` field, the paths/patterns specified here are filtered from the files matched by \`include\`. See the
[Configuration Files guide](${includeGuideLink}) for details.`
[Configuration Files guide](${includeGuideLink}) for details.
`
)
.example([["public/**/*", "tmp/**/*"], {}]),
})
Expand Down
15 changes: 9 additions & 6 deletions garden-service/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ import { Module } from "./types/module"
import { Garden } from "./garden"
import { LogEntry } from "./logger/log-entry"
import { sleep } from "./util/util"
import { some, flatten } from "lodash"
import { some } from "lodash"
import { isConfigFilename, matchPath } from "./util/fs"
import Bluebird from "bluebird"
import { InternalError } from "./exceptions"

// How long we wait between processing added files and directories
const DEFAULT_BUFFER_INTERVAL = 500
const DEFAULT_BUFFER_INTERVAL = 400

export type ChangeHandler = (module: Module | null, configChanged: boolean) => Promise<void>

Expand Down Expand Up @@ -60,6 +60,7 @@ export class Watcher {
if (this.watcher) {
this.log.debug(`Watcher: Clearing handlers`)
this.watcher.removeAllListeners()
this.watcher.close()
delete this.watcher
}
}
Expand Down Expand Up @@ -87,17 +88,19 @@ export class Watcher {
// See https://github.com/garden-io/garden/issues/1269.
// TODO: see if we can extract paths from dotignore files as well (we'd have to deal with negations etc. somehow).
const projectExcludes = this.garden.moduleExcludePatterns.map((p) => resolve(this.garden.projectRoot, p))
const moduleExcludes = flatten(this.modules.map((m) => (m.exclude || []).map((p) => resolve(m.path, p))))
// TODO: filter paths based on module excludes as well
// (requires more complex logic to handle overlapping module sources).
// const moduleExcludes = flatten(this.modules.map((m) => (m.exclude || []).map((p) => resolve(m.path, p))))

this.watcher = watch(this.paths, {
ignoreInitial: true,
ignorePermissionErrors: true,
persistent: true,
awaitWriteFinish: {
stabilityThreshold: 500,
pollInterval: 200,
stabilityThreshold: 300,
pollInterval: 100,
},
ignored: [...projectExcludes, ...moduleExcludes],
ignored: [...projectExcludes],
})

this.watcher
Expand Down
2 changes: 1 addition & 1 deletion garden-service/test/mocha.opts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
--exclude test/,src/,.garden
--watch-extensions js,json,pegjs
--reporter spec
--timeout 20000
--timeout 3000
--exit
build/test/setup.js
build/test/unit/**/*.js

0 comments on commit 6d30b7c

Please sign in to comment.