Support --watching and auto-reloading binary upon changes

[cratelyn]

During development, it's extremely convenient to have the service automatically reload on change. This is a tracking issue for implementing "watching" functionality.

---

Open Questions:

• It might make sense to have it be the default behavior, perhaps with the ability to explicitly disable it?
• Should Viceroy, or the Fastly CLI be responsible for watching files and reloading upon changes? How should the list of files to watch be specified, given that different languages must be supported? (cc: @Integralist)

Originally reported by @tschneidereit; cc: @aturon, @peterbourgon.

[Integralist]

I'm of the mind that this is outside the responsibility of the CLI, but I'm confused by the practicality of how it would work if Viceroy alone handled reloading of itself.

To elaborate:
The CLI allows a user to spawn an instance of Viceroy, but if Viceroy needs to be reloaded due to some code changes, then the CLI would need to know about that reloading in some way so it can handle termination properly.

Currently, we spawn an instance of Viceroy and within that execution flow we configure signal monitoring so that if the user issues a SIGINT or SIGTERM from their shell, then we catch it and kill the Viceroy process. This causes our call to cmd.Exec() to fail and we inspect the returned error for either of the two signals.

So I imagine if Viceroy was to reload itself, then maybe that could use a particular signal or way of indicating to the CLI that a reload occurred for the CLI to then handle tracking the new (i.e. reloaded) Viceroy process so it can be stopped/cleaned-up whenever the user tries sending SIGINT/SIGTERM.

Thoughts? Did any of what I said actual make any sense? 🙂

[peterbourgon]

I suspect if Viceroy itself watched for Wasm changes, it would do so via fsnotify-type stuff and reload without restarting, rather than via user/process signals.

If the CLI would do it, you'd probably do the same way, terminating and restarting the Viceroy child process without exiting the CLI parent process. Or maybe sending SIGHUP to Viceroy, if it understands that one.

[phamann]

My gut feeling is this is functionality going to have to live in the CLI and not Viceroy if we want intelligent recompilation when a user saves a source file, as Viceroy doesn't and shouldn't know how to compile Wasm binaries from a magnitude of source languages - which the CLI does.

Unless the only file being watched was the input binary to Viceroy (i.e. bin/main.wasm), if so, it should live within Viecroy and use fsnotify as @peterbourgon suggests allowing the CLI to remain a simple wrapper over the underlying process.

[williamoverton]

Just an FYI:

In the JS ecosystem there are common tools for this kind of problem, the most popular being nodemon. This is how I am currently testing:

npx nodemon --watch src -e "tsx,jsx,js,ts,json,css,toml" --exec "npm run build && viceroy bin/index.wasm"

Nodemon automatically stops viceroy, runs a build and starts viceroy again each time a file is changed.

[peterbourgon]

In the JS ecosystem there are common tools for this kind of problem

The OS itself has a tool for this purpose: inotify, or it's portable and user-friendly wrapper, fswatch. For example, this will launch Viceroy when the main.wasm is changed:

brew install fswatch
fswatch -o main.wasm | xargs -n1 viceroy

Or, rebuild and re-launch when any file is changed.

fswatch -or . | xargs -n1 sh -c 'fastly compute build && viceroy main.wasm'

The sky's the limit 🌞

And, to give a bit of context to my previous comments, this why I tend to believe auto-reloading functionality is out of scope for tools like the CLI. The capability already exists in the OS, in a relatively straightforward form, and it works for everything!

[mgattozzi]

If the decision ends up being "put the watching capability into Viceroy" instead of the CLI (I don't have any preference here) it should be noted that the notify crate is cross platform and allows watching file directories for changes on Windows/Linux/OSX so the functionality could be built into Viceroy to reload updated wasm files, but as said before it'd mean Viceroy would have to be "language convention aware".

[triblondon]

In a recent devrel/ECP sync meeting, the devrel team suggested that we go ahead and build a --watch flag for the CLI. This is being tracked as DEVEX-132

[mgattozzi]

@triblondon does this mean the CLI would take care of this and that we should close this Viceroy ticket or is there work needed to be done in Viceroy to allow that functionality to be built into the CLI?

[JakeChampion]

@triblondon does this mean the CLI would take care of this and that we should close this Viceroy ticket or is there work needed to be done in Viceroy to allow that functionality to be built into the CLI?

The fastly cli now has a watch flag for this use-case, which I think means we can close this issue (https://developer.fastly.com/reference/cli/compute/serve/)
fastly compute serve --watch

[Integralist]

As @JakeChampion has mentioned, the CLI already implements a --watch flag so this ticket can be closed.