Add Persistent Task Invalidation #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
berfarah
force-pushed
the
bf/persistent-invalidation-cache
branch
from
a0e379f to
9ed212f
Compare
berfarah
force-pushed
the
bf/persistent-invalidation-cache
branch
9 times, most recently
from
7827992 to
1966616
Compare
stephen
reviewed
Nov 2, 2018
| @@ -83,7 +83,7 @@ func Run(tasks []*Task, options ...RunOption) { | |||
| } | |||
|
|
|||
| if len(tasks) == 0 { | |||
| panic(fmt.Errorf("no tasks specified")) | |||
| log.Fatalln("No tasks specified") | |||
There was a problem hiding this comment.
nit: consistent lowercasing?
There was a problem hiding this comment.
I'm for consistently upcasing?
There was a problem hiding this comment.
i wish we didn't change it. either way, these aren't consistent now
cache/cache.go
Outdated
| } | ||
|
|
||
| func getCacheFilePath(dir string) string { | ||
| hashedName := strings.Replace(dir, "/", "%", -1) |
There was a problem hiding this comment.
how about replacing any non-alphanum with -?
There was a problem hiding this comment.
regexp.MustCompile(`[^A-Za-z\-_]`)
There was a problem hiding this comment.
Could do that - I figured slashes would be the only dangerous thing though. NSO
We will need the output of these functions in order to get the beginnings of our cache going, since our cache is based on doing a git diff against a hash + manual hashing of uncommitted files.
berfarah
force-pushed
the
bf/persistent-invalidation-cache
branch
2 times, most recently
from
1fc4a09 to
d672483
Compare
willhug
reviewed
Nov 5, 2018
cache/md5sum.go
Outdated
| "path/filepath" | ||
| ) | ||
|
|
||
| // hashSum takes in a file or directory and hashes based on filename + modified time. |
There was a problem hiding this comment.
This hashes every directory on the path as well right?
There was a problem hiding this comment.
Sorta - it does a hashsum of all the files within a directory OR a hash on a single filename
berfarah
force-pushed
the
bf/persistent-invalidation-cache
branch
from
d672483 to
629e136
Compare
willhug
reviewed
Nov 5, 2018
cache/snapshot.go
Outdated
| return &snapshot, nil | ||
| } | ||
|
|
||
| func (s snapshot) hashFor(path string) string { |
|
|
||
| // Only mark committed files as dirty (uncommitted files are handled below). | ||
| for _, f := range diffFiles { | ||
| if previous.hashFor(f) == "" { |
cache/snapshot.go
Outdated
| // If the file isn't currently uncommitted, rehash. | ||
| md5 := current.hashFor(file.Path) | ||
| if md5 == "" { | ||
| md5, _ = c.HashFunc(file.Path) |
There was a problem hiding this comment.
can we print out this error? Or log it somewhere?
cache/snapshot.go
Outdated
| } | ||
|
|
||
| // write records the current state to a file. | ||
| func (c *snapshotter) Write(ctx context.Context, path string) error { |
| } | ||
| } | ||
|
|
||
| func (c *Cache) Start(ctx context.Context, opt shell.RunOption) error { |
|
|
||
| const CachePath = ".cache/taskrunner" | ||
|
|
||
| var CacheDir = path.Join(os.Getenv("HOME"), CachePath) |
There was a problem hiding this comment.
do we want this cached globally? Or do we want one per repo?
There was a problem hiding this comment.
I moved the logic for that out - it will be one per repo
| return false | ||
| } | ||
| for _, f := range c.dirtyFiles { | ||
| if taskrunner.IsTaskSource(task, f) { |
There was a problem hiding this comment.
IsTaskSource doesn't have a comment. Can we add one?
| @@ -7,28 +7,30 @@ import ( | |||
| "github.com/samsarahq/taskrunner/watcher" | |||
| ) | |||
|
|
|||
| func IsTaskSource(task *Task, path string) (matches bool) { | |||
willhug
approved these changes
Nov 5, 2018
This function works for both files and directories. We are choosing to start with a hash off of filename and modified time for now, but can make this include contents if we need to make the cache more resilient.
These snapshots are comparable across time and will determine invalidated files, allowing Taskrunner to determine whether a task needs to re-run on boot. A diff against a previous commit is preferred. For uncommitted (and new) files, the hash is compared to a hash in the previous snapshot. If there was no hash before, the file is marked as changed.
Since we will be using this functionality to determine whether a task should be invalidated between task runs, it makes sense to expose this as public API. Leaving this off of the Task struct for now, but could easily move this to be a method on Task.
With this feature, tasks can remain valid between taskrunner executions if the task's dependent files have not changed. An example use-case of this is `yarn install`, which we can safely assume shouldn't run again unless `package.json`/`yarn.lock` have changed.
Many of our panics aren't that exceptional and should be normal errors to a user. This should help reduce noise when looking at Taskrunner errors.
berfarah
force-pushed
the
bf/persistent-invalidation-cache
branch
from
629e136 to
58dd6f1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this feature, tasks can remain valid between taskrunner executions if the task's dependent files have not changed.
An example use-case of this is
yarn install, which we can safely assume shouldn't run again unlesspackage.json/yarn.lockhave changed.This requires a git repository to be in place to work.