Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: run local --watch flag #5413

Merged
merged 28 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9b15223
write file package to track filesystem changes
Oct 26, 2023
3bb3f2f
run local --watch flag
Oct 26, 2023
e265dd5
reverse accidental line spacing
Oct 26, 2023
423fafd
handle returned errors
Oct 26, 2023
61bcafc
fix merge conflict
Oct 26, 2023
14eb108
Merge branch 'mainline' into cli/run-local-watch-flag
CaptainCarpensir Oct 26, 2023
8a7d8a2
reorder test calls
Oct 26, 2023
9701f70
Merge branch 'cli/run-local-watch-flag' of https://github.com/Captain…
Oct 26, 2023
21d6605
simplify debounce
Oct 27, 2023
8ea2c38
removed unused parameter
Oct 27, 2023
142bc12
address feedback
Oct 30, 2023
80aa093
dont update for hidden files and folders
Nov 3, 2023
3ccfae2
watcher integration test
Nov 3, 2023
0364052
localintegration build tag
Nov 3, 2023
64dffc8
Merge branch 'mainline' into cli/run-local-watch-flag
CaptainCarpensir Nov 3, 2023
52ca8e7
fix merge errors
Nov 3, 2023
497df05
remove race conditions
Nov 3, 2023
b09f63a
fix merge change causing tests to time out
Nov 3, 2023
e0c82a8
buffer watcher for tests
Nov 3, 2023
28c34b7
make integration test non-blocking on failure
Nov 3, 2023
edcc10f
two events
Nov 3, 2023
563bdc3
address adi feedback
Nov 6, 2023
c361a7d
make watcher close not handle outstanding events/errors
Nov 6, 2023
6439229
Update internal/pkg/cli/file/watch.go
CaptainCarpensir Nov 7, 2023
e4c2f41
Merge branch 'mainline' into cli/run-local-watch-flag
CaptainCarpensir Nov 7, 2023
193b84e
fix merge changes
Nov 7, 2023
6349e0c
address wanxian feedback, fix tests
Nov 7, 2023
dd9dd0b
Merge branch 'mainline' into cli/run-local-watch-flag
mergify[bot] Nov 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/dustin/go-humanize v1.0.1
github.com/fatih/color v1.15.0
github.com/fatih/structs v1.1.0
github.com/fsnotify/fsnotify v1.7.0
github.com/golang/mock v1.6.0
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/google/uuid v1.4.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs=
github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo=
github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
Expand Down
38 changes: 38 additions & 0 deletions internal/pkg/cli/file/filetest/watchertest.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package filetest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's used in run_local_test.go but maybe the package name should be changed.


import "github.com/fsnotify/fsnotify"

// Double is a test double for file.RecursiveWatcher
type Double struct {
EventsFn func() <-chan fsnotify.Event
ErrorsFn func() <-chan error
}

// Add is a no-op for Double.
func (d *Double) Add(string) error {
return nil
}

// Close is a no-op for Double.
func (d *Double) Close() error {
return nil
}

// Events calls the stubbed function.
func (d *Double) Events() <-chan fsnotify.Event {
if d.EventsFn == nil {
return nil
}
return d.EventsFn()
}

// Errors calls the stubbed function.
func (d *Double) Errors() <-chan error {
if d.ErrorsFn == nil {
return nil
}
return d.ErrorsFn()
}
13 changes: 13 additions & 0 deletions internal/pkg/cli/file/hidden.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//go:build !windows

// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package file

import "path/filepath"

// IsHiddenFile returns true if the file is hidden on non-windows.
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
func IsHiddenFile(filename string) (bool, error) {
return filepath.Base(filename)[0] == '.', nil
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
}
21 changes: 21 additions & 0 deletions internal/pkg/cli/file/hidden_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package file

import (
"syscall"
)

// IsHiddenFile returns true if the file is hidden on windows.
func IsHiddenFile(filename string) (bool, error) {
pointer, err := syscall.UTF16PtrFromString(filename)
if err != nil {
return false, err
}
attributes, err := syscall.GetFileAttributes(pointer)
if err != nil {
return false, err
}
return attributes&syscall.FILE_ATTRIBUTE_HIDDEN != 0, nil
}
130 changes: 130 additions & 0 deletions internal/pkg/cli/file/watch.go
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package file

import (
"io/fs"
"os"
"path/filepath"

"github.com/fsnotify/fsnotify"
)

// RecursiveWatcher wraps an fsnotify Watcher to recursively watch all files in a directory.
type RecursiveWatcher struct {
fsnotifyWatcher *fsnotify.Watcher
done chan struct{}
closed bool
events chan fsnotify.Event
errors chan error
}

// NewRecursiveWatcher returns a RecursiveWatcher which notifies when changes are made to files inside a recursive directory tree.
func NewRecursiveWatcher() (*RecursiveWatcher, error) {
watcher, err := fsnotify.NewWatcher()
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

rw := &RecursiveWatcher{
events: make(chan fsnotify.Event),
errors: make(chan error),
fsnotifyWatcher: watcher,
done: make(chan struct{}),
closed: false,
}

go rw.start()

return rw, nil
}

// Add recursively adds a directory tree to the list of watched files.
func (rw *RecursiveWatcher) Add(path string) error {
if rw.closed {
return fsnotify.ErrClosed
}
if err := filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return rw.fsnotifyWatcher.Add(path)
}
return nil
}); err != nil {
return err
}
return nil
}

// Remove recursively removes a directory tree from the list of watched files.
func (rw *RecursiveWatcher) Remove(path string) error {
if rw.closed {
return nil
}
if err := filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return rw.fsnotifyWatcher.Remove(path)
}
return nil
}); err != nil {
return err
}
return nil
}

// Events returns the events channel.
func (rw *RecursiveWatcher) Events() <-chan fsnotify.Event {
return rw.events
}

// Errors returns the errors channel.
func (rw *RecursiveWatcher) Errors() <-chan error {
return rw.errors
}

// Close closes the RecursiveWatcher.
func (rw *RecursiveWatcher) Close() error {
rw.closed = true
close(rw.done)
return rw.fsnotifyWatcher.Close()
}

func (rw *RecursiveWatcher) start() {
for {
select {
case event := <-rw.fsnotifyWatcher.Events:
info, err := os.Stat(event.Name)
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
break
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
}
if info.IsDir() {
switch event.Op {
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
case fsnotify.Create:
err := rw.Add(event.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like event.Name is relative to the "input" (https://pkg.go.dev/github.com/fsnotify/fsnotify#Event). Does rw.Add expects an absolute path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, rw.Add calls the underlying Add function in the fsnotify.Watcher. This function can be both absolute or relative. I think this can only become an issue if the relative position being called from changes. This should error out though, and because --watch programmatically calls this, it's just reusing the directory provided by ws.Path(), so we'll never run into this issue here anyways.

if err != nil {
rw.errors <- err
}
case fsnotify.Remove:
err := rw.Remove(event.Name)
if err != nil {
rw.errors <- err
}
}
} else {
rw.events <- event
}
case err := <-rw.fsnotifyWatcher.Errors:
rw.errors <- err
case <-rw.done:
close(rw.events)
close(rw.errors)
return
}
}
}
2 changes: 2 additions & 0 deletions internal/pkg/cli/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const (
portOverrideFlag = "port-override"
envVarOverrideFlag = "env-var-override"
proxyFlag = "proxy"
watchFlag = "watch"

// Flags for CI/CD.
githubURLFlag = "github-url"
Expand Down Expand Up @@ -322,6 +323,7 @@ Format: [container]:KEY=VALUE. Omit container name to apply to all containers.`
portOverridesFlagDescription = `Optional. Override ports exposed by service. Format: <host port>:<service port>.
Example: --port-override 5000:80 binds localhost:5000 to the service's port 80.`
proxyFlagDescription = `Optional. Proxy outbound requests to your environment's VPC.`
watchFlagDescription = `Optional. Watch changes to local files and restart containers when updated.`

svcManifestFlagDescription = `Optional. Name of the environment in which the service was deployed;
output the manifest file used for that deployment.`
Expand Down
Loading
Loading