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 7, 2019
1 parent 95cbd21 commit 989e9f2
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 9 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
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
8 changes: 5 additions & 3 deletions garden-service/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ 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"
Expand Down Expand Up @@ -87,7 +87,9 @@ 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,
Expand All @@ -97,7 +99,7 @@ export class Watcher {
stabilityThreshold: 500,
pollInterval: 200,
},
ignored: [...projectExcludes, ...moduleExcludes],
ignored: [...projectExcludes],
})

this.watcher
Expand Down

0 comments on commit 989e9f2

Please sign in to comment.