Following: `auto_reload` support for Windows

Moving forward Feature Requests
#1

I’ll paste here my response to #1346 and would like everyone to discuss using this tool, if possible :wink:

@abuckenheimer @yunstanford I know I have made some tools in the past that were way more complex because they required either binary or byte-compiled code to be hot-replaced, either in Windows or Linux or (whatever the name of that embed machine made in hell). This should not be complicated, with my first idea being wrapping the server stuff and launching a second process with multiprocessing.spawn, that would certainly work on Linux and Windows. The main process would watch for file changes and restart (kill+spawn) the new server on some events (either by using inotify in Linux or simple polling in Windows, though I think there might be something better / already done for that).

Also, I agree with the idea that this should be put into a plugin and not within Sanic since it is just a tool, not a production specific feature.

#2

+1 for the tool approach. I’m generally in favor of breaking things out of the core package where appropriate and possible. I have a feeling i may be more aggressive on this than others (I’d like to see websockets also moved into its own package).

1 Like
#3

Me too. There’s nothing better than decoupling to really test your code :relaxed:

2 Likes
#4

I raised the request, and am chuffed to see you thinking hard about how to do it.

One thought on the possible direction that you mention:

I think Flask takes this approach, and one downside with it is that breakpoints no longer work (at least not in VS Code). I guess that’s because VS Code attaches to the initial process and not the spawned one.

No idea if it’s possible to make breakpoints work and have auto-reloading, but if you can manage it that would put you one up on Flask (no idea if that’s a motivation :), Sanic already has async which blows it out of the water!)

1 Like
#5

Well, indeed this is one of the most possible solutions; as you mentioned it is the approach other frameworks like Flask already use as well. The thing regarding VS Code, well … Would be to make it identify the new launched process. IDK if that’s possible (I use VS Code but just to code, I do debugging in the console, the old fashioned way). Perhaps a different SIGNAL instead of kill + spawn? :thought_balloon:

I think the main order of priority here would be:

  1. Check if the proposed solution works for auto_reload in Windows (I really need a Windows VM for this) and Linux;
  2. If (and only if) some extra time for this is available, then figure out how some of these IDEs tracks children processes (not only VS Code, it will be certainly pointless to dig into VS Code and pretend other IDEs aren’t even there for this feature).

It certainly is! :sunglasses:

2 Likes
#6

After a brief research, I found out that PyCharm, PyDev and VSCode Python uses the same debugger implementation under the hood: pydevd. I even found an example of a Django middleware that imports pydevd and hooks itself into PyCharm.

Even though I don’t think this must be a requirement, a future plugin implementing auto_reload could optionaly search for pydevd and spawn processes that self-attach to a running process of pydevd - I’ll just have to learn how to do it :joy:

Also, pydevd-reload can give us some hints on how to accomplish that.

I don’t think I’ll have the time to do this right now, but it can certainly enter our roadmap :+1:

1 Like
Sanic process lingering when debug stopped in VS Code
#7

Even though I’m a Sublime guy, that sounds pretty awesome.

1 Like
#8

Yeah, I do like Sublime as well but I didn’t want to pay for another license (I had bought ST2 some time ago) and I really found out VSCode to be even better with some sweet Python tools and integrations (even black works inside VSCode) … Also, a lot of extensions in VSCode supports the native config of your environment (let’s say you have a setup.cfg with your isort or flake8 configs in there, it just works. With ST, I had to put the config inside and if changing a project would require another config, I had to change everything else (again) … I don’t know if that changed, if so I might give it a try (after all, VSCode is made with Electron and memory is not cheap :confused: ).

#9

So, let me share what I discovered so far regarding the auto_reload and how to accomplish a “better result”.

First, to have it working on Windows, I do not pretend to reinvent the wheel: even though inotify is awesome under Linux, it is just for Linux. So, I made a simple async wrap for watchdog and then I don’t have to bother by implementing my own “FS watcher” for Linux, Windows and etc. So far so good.

The second point is where I need your input: I was thinking in passing only the app instance to this helper, like watch(app), but then I’ll have to check for all loaded modules (like it’s done today by Sanic, which I think it’s an overkill) and also try to identify from where it was first called, iterating over inspect.stack() to find out where watch(app) was used, but I think this is a long shot and might be error prone.

The solution I see is, if feasable, pass the app and the source directory which the auto_reloader should check for modified files. Examples:

  1. A simple function call, like:
    watch(app[, "/my/path/to/project"])
  2. A factory:
    watcher = create_watcher("/my/path/to/project" [or None])
watcher.run(app)
  3. A bloated class:
    class MyWatcher(Watcher):
    def get_path(self):
        return "/my/path/to/project" [or None]
    def get_app(self):
        return self.app  # example
  4. A command line utility, which I can use the source folder of myfile in the example bellow:
    $ python -m newwatchername --app=myfile:app [--source-path=/my/path/to/project]

Let me know your thoughts :wink:

#10

#4 has the benefit of looking somewhat Django- like with its own development runserver. This might have a further implementation if we ever wanted to add some sort of development diagnostics or error catching.

And, of course, flask has its own cli built in with click. I once played around with an argparse wrapper that instantiated a loop to be able to tie into some async/await code in use in my sanic app: https://github.com/ahopkins/asynccli

I’ll try and take a more in depth look at this myself. But I support the watchdog wrapper idea.

#11

Yes, but it kind resembles the usage of any of the three other examples (under the hood), being my favorite one the factory method (because it is simple as that). Both cases will end up with the same codebase and it’ll be a choice of the developer to launch the application using the command line or directly within the code.

But, one thing is for certain: I don’t think we’ll be able to just pass a parameter for app.run to start a development server … This will be apart Sanic.

#12

Would bundling in a tool like watchexec be an option? What are the pros/cons of bundling in a tool like this?

#13

@mackeyja92 I think it would be an option, although I think the idea was to use: watchdog (which has the benefit of being a python package).

Does watchexec provide any benefits over watchdog?

#14

I’d prefer watchdog as well, solely for the reason of being a python implementation.

#15

Thanks for your interest, @mackeyja92! The reason we are using watchdog is just for the inner works of the tool (it’ll be used internally by this new testing tool) and, since it is planned to work with Python projects (like Sanic) and provide better integration with development environments (using pydevd), it only makes sense to use a Python dependency (that has an API acessible by Python, to be more precise).

Check out the issue that generated this discussion here for more background info :wink:

#16

Awesome! Thanks for the info. I am actually not that informed on these kinds of tools. I simply saw watchexec the other day and then happened to run into the issue where the debugger doesn’t work when using autoreload.

#17

No problem! In the end, this tool will end up being something like watchexec, except for the target audience (and some sugar here and there) :slight_smile:

Call to arms: Testing coverage