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

Add a spin dev command #1102

Closed
calebschoepp opened this issue Feb 2, 2023 · 9 comments
Closed

Add a spin dev command #1102

calebschoepp opened this issue Feb 2, 2023 · 9 comments
Labels
enhancement New feature or request open for comment

Comments

@calebschoepp
Copy link
Collaborator

calebschoepp commented Feb 2, 2023

I propose that we add a new top level command spin dev. This command seeks to address two issues we have with the dev inner loop: a desire for directory watching/hot reloading (#148 #324) and confusion around where logs go (#993).

When you run spin dev the following would happen:

  1. spin build is run.
  2. spin up --follow-all is run.
  3. The application directory is watched (recursively) to see if there any file changes and if so then steps 1 and 2 are repeated.

spin dev would become the new command designed for the dev inner loop experience. It would make finding logs trivial because they'll always be dumped to stdout. It would also give us hot reloading. We can also turn on other things like wasm_backtrace_details (#389) that we would only want turned on in a dev environment.

The new user flow would become something like:

  1. Install spin
  2. spin dev
  3. spin deploy

spin build and spin up would then become more focused on the CI/CD and production use cases. This means we can keep the performant default of writing logs to disk for spin up. It also means we don't have to clutter up these commands with flags for hot reloading. For example it would be confusing for them to have to do two flags for the dev workflow spin build --up --watch.

Note: One potential issue with this approach is that hot reloading can be a slippery slope of work and weird bugs. Perhaps this isn't a feature we want to maintain. However, IMO hot reloading is critical to the DX.

Note: When I refer to hot reloading I'm talking about completely rerunning spin build when any file changes. I'm not talking about a spin server grabbing new web assembly artifacts without stopping.

@lann
Copy link
Collaborator

lann commented Feb 2, 2023

I like the idea overall, but I would tweak the rebuild behavior a bit:

  • By default, only watch the explicit inputs listed in spin.toml, i.e. component.source, component.files, and spin.toml itself.
  • Explicitly list patterns to watch for build in the [component.build] section, e.g. watch = ["component/src/**/*"]. These would be relative to build.workdir.

Users can still opt-in to "watch everything" with "**/*", but in my experience that can be very frustrating when combined with e.g. rust-analyzer rebuilding stuff in the background all the time.

@lann
Copy link
Collaborator

lann commented Feb 2, 2023

Another option would be to delegate rebuilding "watch" behavior to a build.watch_command which would be started and stopped with spin dev and would be expected to update e.g. component.source itself (which Spin would be "watching").

@calebschoepp
Copy link
Collaborator Author

By default, only watch the explicit inputs listed in spin.toml, i.e. component.source, component.files, and spin.toml itself.
I don't quite understand. Are you saying only restart spin up when explicit spin.toml inputs change. And then only rerun spin build when [component.build.watch] pattern matching files are modified -- thus triggering spin up to restart too? Is that in effect not the same behaviour?

Another option would be to delegate rebuilding "watch" behavior to a build.watch_command which would be started and stopped with spin dev and would be expected to update e.g. component.source itself (which Spin would be "watching").

This would simplify the scope and I see it's merits, but I would still advocate for it to be built into Spin.

CCing community members for visibility on this issue: @joepio @mreferre

@lann
Copy link
Collaborator

lann commented Feb 2, 2023

By default, only watch the explicit inputs listed in spin.toml, i.e. component.source, component.files, and spin.toml itself.

I don't quite understand. Are you saying only restart spin up when explicit spin.toml inputs change. And then only rerun spin build when [component.build.watch] pattern matching files are modified -- thus triggering spin up to restart too? Is that in effect not the same behaviour?

This would be an alternative to a default of "rerunning spin build when any file changes".

Edit: Or maybe the confusion is from my formatting. The two bullet points aren't two options; they go together.

@itowlson
Copy link
Contributor

itowlson commented Feb 2, 2023

This feels like not a good idea to me - I don't see that we need two separate commands here. spin up should provide the local dev experience. Improvements to the local dev experience should go there (when we can agree what they are!). People who disagree can set environment variables or use shell scripts or a hypothetical preferences command or whatever - but we don't need a whole separate command, with all the documentation and testing burden that comes with that, to stitch together build -> run with favourite options -> watch.

Where there are things that need to be different in dev use and CI/production/server use, it should be the "production" mode that is on a flag, because a server doesn't care how long the command line is.

If we really do feel that we have no choice but to have two separate commands for dev and CI/prod modes, I'd prefer the new command to be the "prod mode" one. There's a lot of messaging already behind spin up and I don't see the value in changing that.

#include <std-disclaimer.h>

@lann
Copy link
Collaborator

lann commented Feb 2, 2023

Having a separate "prod mode" subcommand (spin serve?) would help with a future issue I've been thinking about: how to deal with multiple trigger types in an app. In a dev env spin up can happily start them all, but in production you almost certainly want to manage each trigger type separately (e.g. you really don't want to run multiple timer trigger instances).

tldr: I agree with @itowlson here.

@itowlson
Copy link
Contributor

itowlson commented Feb 2, 2023

I wondered about spin serve but wasn't sure if that reflected scenarios like queue subscription or cron jobs. But of course we are a long way from having to settle on a name!

@calebschoepp
Copy link
Collaborator Author

@itowlson I hear you on keeping the branding around spin up. I'm ambivalent on whether the new command is for the ci/prod mode or dev mode -- I just think that we should separate the dev and ci/prod use cases at the command level. As spin gets increasingly more complex the amount of flags you'll want to pass to spin up to make it work for your dev or prod use case will get out of hand.

Given all that perhaps the new way to look at it would be:
spin build -> Same old
spin up -> Focused on inner dev loop, offer a --watch flag, default logs to stdout, etc.
spin serve (or your preferred command name) -> Focused on prod/ci use cases, log to file, etc.

@github-project-automation github-project-automation bot moved this to 🆕 Triage Needed in Spin Triage Feb 3, 2023
@michelleN michelleN added enhancement New feature or request open for comment labels Feb 3, 2023
@michelleN michelleN moved this from 🆕 Triage Needed to 📋 Investigating / Open for Comment in Spin Triage Feb 3, 2023
@calebschoepp
Copy link
Collaborator Author

Closing this because we've made a number of changes that have made this irrelevant, in particular:

@github-project-automation github-project-automation bot moved this from 📋 Investigating / Open for Comment to ✅ Done in Spin Triage Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request open for comment
Projects
Status: Done
Development

No branches or pull requests

4 participants