From 546a4d7e46ec4bf0f7d19e12a4a91e2f9bf39fc4 Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Fri, 17 Nov 2023 14:51:10 -0600 Subject: [PATCH] feat: prefer remote taskfiles over cached ones (#1345) * feat: prefer remote taskfiles over cached ones * feat: implemented cache on network timeout * feat: --download always downloads, but never executes tasks * feat: --timeout flag * fix: bug with timeout error handling * chore: changelog --- CHANGELOG.md | 15 +++-- cmd/task/task.go | 13 ++++- docs/docs/experiments/remote_taskfiles.md | 21 +++---- errors/errors.go | 1 + errors/errors_taskfile.go | 24 ++++++++ setup.go | 1 + task.go | 1 + taskfile/read/taskfile.go | 71 +++++++++++++---------- 8 files changed, 99 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3410ef613f..e762d47bbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,16 @@ ## Unreleased +- The + [Remote Taskfiles experiment](https://taskfile.dev/experiments/remote-taskfiles) + now prefers remote files over cached ones by default (#1317, #1345 by @pd93). +- Added `--timeout` flag to the + [Remote Taskfiles experiment](https://taskfile.dev/experiments/remote-taskfiles) + (#1317, #1345 by @pd93). - Fix bug where dynamic `vars:` and `env:` were being executed when they should actually be skipped by `platforms:` (#1273, #1377 by @andreynering). -- Fix `schema.json` to make `silent` valid in `cmds` that use `for` (#1385, #1386 by @iainvm). +- Fix `schema.json` to make `silent` valid in `cmds` that use `for` (#1385, + #1386 by @iainvm). - Add new `--no-status` flag to skip expensive status checks when running `task --list --json` (#1348, #1368 by @amancevice). @@ -12,7 +19,7 @@ - Enabled the `--yes` flag for the [Remote Taskfiles experiment](https://taskfile.dev/experiments/remote-taskfiles) - (#1344 by @pd93). + (#1317, #1344 by @pd93). - Add ability to set `watch: true` in a task to automatically run it in watch mode (#231, #1361 by @andreynering). - Fixed a bug on the watch mode where paths that contained `.git` (like @@ -26,8 +33,8 @@ exists to detect recursive calls, but will be removed in favor of a better algorithm soon (#1321, #1332). - Fixed templating on descriptions on `task --list` (#1343 by @blackjid). -- Fixed a bug where precondition errors were incorrectly being printed when - task execution was aborted (#1337, #1338 by @sylv-io). +- Fixed a bug where precondition errors were incorrectly being printed when task + execution was aborted (#1337, #1338 by @sylv-io). ## v3.30.1 - 2023-09-14 diff --git a/cmd/task/task.go b/cmd/task/task.go index dd5f26f695..b0edda2991 100644 --- a/cmd/task/task.go +++ b/cmd/task/task.go @@ -75,6 +75,7 @@ var flags struct { experiments bool download bool offline bool + timeout time.Duration } func main() { @@ -150,6 +151,7 @@ func run() error { if experiments.RemoteTaskfiles { pflag.BoolVar(&flags.download, "download", false, "Downloads a cached version of a remote Taskfile.") pflag.BoolVar(&flags.offline, "offline", false, "Forces Task to only use local or cached Taskfiles.") + pflag.DurationVar(&flags.timeout, "timeout", time.Second*10, "Timeout for downloading remote Taskfiles.") } pflag.Parse() @@ -235,6 +237,7 @@ func run() error { Insecure: flags.insecure, Download: flags.download, Offline: flags.offline, + Timeout: flags.timeout, Watch: flags.watch, Verbose: flags.verbose, Silent: flags.silent, @@ -270,6 +273,12 @@ func run() error { return err } + // If the download flag is specified, we should stop execution as soon as + // taskfile is downloaded + if flags.download { + return nil + } + if listOptions.ShouldListTasks() { foundTasks, err := e.ListTasks(listOptions) if err != nil { @@ -298,9 +307,7 @@ func run() error { } // If there are no calls, run the default task instead - // Unless the download flag is specified, in which case we want to download - // the Taskfile and do nothing else - if len(calls) == 0 && !flags.download { + if len(calls) == 0 { calls = append(calls, taskfile.Call{Task: "default", Direct: true}) } diff --git a/docs/docs/experiments/remote_taskfiles.md b/docs/docs/experiments/remote_taskfiles.md index 56ecdc8191..dcd9ebaff6 100644 --- a/docs/docs/experiments/remote_taskfiles.md +++ b/docs/docs/experiments/remote_taskfiles.md @@ -74,16 +74,17 @@ you are doing. ## Caching & Running Offline -If for whatever reason, you don't have access to the internet, but you still -need to be able to run your tasks, you are able to use the `--download` flag to -store a cached copy of the remote Taskfile. - - - -If Task detects that you have a local copy of the remote Taskfile, it will use -your local copy instead of downloading the remote file. You can force Task to -work offline by using the `--offline` flag. This will prevent Task from making -any calls to remote sources. +Whenever you run a remote Taskfile, the latest copy will be downloaded from the +internet and cached locally. If for whatever reason, you lose access to the +internet, you will still be able to run your tasks by specifying the `--offline` +flag. This will tell Task to use the latest cached version of the file instead +of trying to download it. You are able to use the `--download` flag to update +the cached version of the remote files without running any tasks. + +By default, Task will timeout requests to download remote files after 10 seconds +and look for a cached copy instead. This timeout can be configured by setting +the `--timeout` flag and specifying a duration. For example, `--timeout 5s` will +set the timeout to 5 seconds. [remote-taskfiles-experiment]: https://github.com/go-task/task/issues/1317 diff --git a/errors/errors.go b/errors/errors.go index 2e98bb8e43..ba87104056 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -18,6 +18,7 @@ const ( CodeTaskfileNotSecure CodeTaskfileCacheNotFound CodeTaskfileVersionNotDefined + CodeTaskfileNetworkTimeout ) // Task related exit codes diff --git a/errors/errors_taskfile.go b/errors/errors_taskfile.go index ac1708e7dc..9dc932f108 100644 --- a/errors/errors_taskfile.go +++ b/errors/errors_taskfile.go @@ -3,6 +3,7 @@ package errors import ( "fmt" "net/http" + "time" ) // TaskfileNotFoundError is returned when no appropriate Taskfile is found when @@ -137,3 +138,26 @@ func (err *TaskfileVersionNotDefined) Error() string { func (err *TaskfileVersionNotDefined) Code() int { return CodeTaskfileVersionNotDefined } + +// TaskfileNetworkTimeout is returned when the user attempts to use a remote +// Taskfile but a network connection could not be established within the timeout. +type TaskfileNetworkTimeout struct { + URI string + Timeout time.Duration + CheckedCache bool +} + +func (err *TaskfileNetworkTimeout) Error() string { + var cacheText string + if err.CheckedCache { + cacheText = " and no offline copy was found in the cache" + } + return fmt.Sprintf( + `task: Network connection timed out after %s while attempting to download Taskfile %q%s`, + err.Timeout, err.URI, cacheText, + ) +} + +func (err *TaskfileNetworkTimeout) Code() int { + return CodeTaskfileNetworkTimeout +} diff --git a/setup.go b/setup.go index afbe88ba2c..ba53a15131 100644 --- a/setup.go +++ b/setup.go @@ -91,6 +91,7 @@ func (e *Executor) readTaskfile() error { e.Insecure, e.Download, e.Offline, + e.Timeout, e.TempDir, e.Logger, ) diff --git a/task.go b/task.go index c646df8a06..d43b1f8033 100644 --- a/task.go +++ b/task.go @@ -46,6 +46,7 @@ type Executor struct { Insecure bool Download bool Offline bool + Timeout time.Duration Watch bool Verbose bool Silent bool diff --git a/taskfile/read/taskfile.go b/taskfile/read/taskfile.go index 5abdd1b9d2..636944869e 100644 --- a/taskfile/read/taskfile.go +++ b/taskfile/read/taskfile.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "runtime" + "time" "gopkg.in/yaml.v3" @@ -37,6 +38,7 @@ func readTaskfile( node Node, download, offline bool, + timeout time.Duration, tempDir string, l *logger.Logger, ) (*taskfile.Taskfile, error) { @@ -51,35 +53,44 @@ func readTaskfile( } } - // If the file is remote, check if we have a cached copy - // If we're told to download, skip the cache - if node.Remote() && !download { - if b, err = cache.read(node); !errors.Is(err, os.ErrNotExist) && err != nil { + // If the file is remote and we're in offline mode, check if we have a cached copy + if node.Remote() && offline { + if b, err = cache.read(node); errors.Is(err, os.ErrNotExist) { + return nil, &errors.TaskfileCacheNotFound{URI: node.Location()} + } else if err != nil { return nil, err } + l.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location()) - if b != nil { - l.VerboseOutf(logger.Magenta, "task: [%s] Fetched cached copy\n", node.Location()) - } - } + } else { - // If the file is remote, we found nothing in the cache and we're not - // allowed to download it then we can't do anything. - if node.Remote() && b == nil && offline { - if b == nil && offline { - return nil, &errors.TaskfileCacheNotFound{URI: node.Location()} - } - } + downloaded := false + ctx, cf := context.WithTimeout(context.Background(), timeout) + defer cf() - // If we still don't have a copy, get the file in the usual way - if b == nil { - b, err = node.Read(context.Background()) - if err != nil { + // Read the file + b, err = node.Read(ctx) + // If we timed out then we likely have a network issue + if node.Remote() && errors.Is(ctx.Err(), context.DeadlineExceeded) { + // If a download was requested, then we can't use a cached copy + if download { + return nil, &errors.TaskfileNetworkTimeout{URI: node.Location(), Timeout: timeout} + } + // Search for any cached copies + if b, err = cache.read(node); errors.Is(err, os.ErrNotExist) { + return nil, &errors.TaskfileNetworkTimeout{URI: node.Location(), Timeout: timeout, CheckedCache: true} + } else if err != nil { + return nil, err + } + l.VerboseOutf(logger.Magenta, "task: [%s] Network timeout. Fetched cached copy\n", node.Location()) + } else if err != nil { return nil, err + } else { + downloaded = true } // If the node was remote, we need to check the checksum - if node.Remote() { + if node.Remote() && downloaded { l.VerboseOutf(logger.Magenta, "task: [%s] Fetched remote copy\n", node.Location()) // Get the checksums @@ -102,24 +113,21 @@ func readTaskfile( } } - // If the hash has changed (or is new), store it in the cache + // If the hash has changed (or is new) if checksum != cachedChecksum { + // Store the checksum if err := cache.writeChecksum(node, checksum); err != nil { return nil, err } + // Cache the file + l.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location()) + if err = cache.write(node, b); err != nil { + return nil, err + } } } } - // If the file is remote and we need to cache it - if node.Remote() && download { - l.VerboseOutf(logger.Magenta, "task: [%s] Caching downloaded file\n", node.Location()) - // Cache the file for later - if err = cache.write(node, b); err != nil { - return nil, err - } - } - var t taskfile.Taskfile if err := yaml.Unmarshal(b, &t); err != nil { return nil, &errors.TaskfileInvalidError{URI: filepathext.TryAbsToRel(node.Location()), Err: err} @@ -137,12 +145,13 @@ func Taskfile( insecure bool, download bool, offline bool, + timeout time.Duration, tempDir string, l *logger.Logger, ) (*taskfile.Taskfile, error) { var _taskfile func(Node) (*taskfile.Taskfile, error) _taskfile = func(node Node) (*taskfile.Taskfile, error) { - t, err := readTaskfile(node, download, offline, tempDir, l) + t, err := readTaskfile(node, download, offline, timeout, tempDir, l) if err != nil { return nil, err }