-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[ENV-44] introduce experimental watch command (skeletton) #10163
Conversation
3f67b19
to
d35ca0d
Compare
Codecov ReportBase: 73.89% // Head: 73.89% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## v2 #10163 +/- ##
=======================================
Coverage 73.89% 73.89%
=======================================
Files 2 2
Lines 272 272
=======================================
Hits 201 201
Misses 60 60
Partials 11 11 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
2146067
to
08d87e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start!
FYI this won't actually watch recursively on the context directory - though not suggesting that be part of this PR as it's gonna be a loooot more code! 😬
More file-watching brain dump:
For Linux, it's still necessary to walk the file tree and add watchers for each directory entry (on start and keep it up-to-date as directories are created/removed at runtime 🫣). Here's the Tilt watcher: https://github.com/tilt-dev/tilt/blob/master/internal/watch/watcher_naive.go (note: this uses tilt-dev/fsnotify fork!)
For Windows, recursive support should be available in the next fsnotify/fsnotify
release: see fsnotify/fsnotify#540 - note that this was inspired by the Tilt fork but uses the more Go-like /...
path suffix instead of a method/option on the watcher itself
For macOS, fsnotify
only supports kqueue
(single files) rather than FSEvents (entire directories). Apple has this to say about them:
File system events are intended to provide notification of changes with directory-level granularity. For most purposes, this is sufficient. In some cases, however, you may need to receive notifications with finer granularity. For example, you might need to monitor only changes made to a single file. For that purpose, the kernel queue (kqueue) notification system is more appropriate.
If you are monitoring a large hierarchy of content, you should use file system events instead, however, because kernel queues are somewhat more complex than kernel events, and can be more resource intensive because of the additional user-kernel communication involved.
Again, to use Tilt as an example, here's how it uses FSEvents: https://github.com/tilt-dev/tilt/blob/master/internal/watch/watcher_darwin.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🥳 🔄
08d87e2
to
6a44f03
Compare
6a44f03
to
97ead7f
Compare
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
97ead7f
to
4569d6d
Compare
experimental: false | ||
experimentalcli: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we need to set the experimentalcli
annotation for this. We removed the "gate" for disabling/hiding those commands, but I guess we could still use the annotation to be used on docs.docker.com. (happy to discuss / brainstorm)
What I did
introduced
watch
command skeletton as an experimental commandwatch is a subcommand for the new (and hidden)
alpha
command, so user can rundocker compose alpha watch
, knowing this is experimental and subject to changes. Once ready for announcement, command will be promoted to top-level compose command.Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did