Switch from `watchgod` to `watchfiles` #1437

samuelcolvin · 2022-04-01T16:49:41Z

See https://watchfiles.helpmanual.io/migrating/.

TL;DR; I've renamed watchgod to watchfiles and rewritten it to use the rust "notify" crate for all the heavy lifting. This PR is a WIP to see if you're willing to move ahead with a change like this.

Still TODO/to answer:

I've changed the logic of BaseReload to try and make StatReload and watchfiles share an interface, are you happy with this?
tests are a fair bit more complicated, are you happy with how I'm implemented/changed them?
I don't see an obvious way to deal with --reload-include and --reload-exclude being paths rather than extensions. Are you happy to drop this functionality? It's much less important now there's little overhead to scanning big directories, if not, we can implement something but it'll be complicated. I think I've dealt with this a287d5c

samuelcolvin · 2022-04-01T21:20:28Z

Would be great if someone could kick off tests?

samuelcolvin · 2022-04-02T07:23:16Z

From https://github.com/encode/uvicorn/settings/actions the below option might save you a lot of time kicking off tests and would be helpful for new contributors.

Kludex · 2022-04-02T07:25:30Z

Ah... I didn't know it was configurable. I'm going ask @tomchristie to enable that over encode. Thanks 🙏

Ping @florimondmanca as you asked about this the other day.

samuelcolvin · 2022-04-02T07:26:49Z

Ah... I didn't know it was configurable. I'm going ask @tomchristie to enable that over encode. Thanks 🙏

Ye, took me some time to find it, it's really helpful.

samuelcolvin · 2022-04-03T08:41:14Z

I would love feedback on this, but please don't merge. I'd like to make some more changes once samuelcolvin/watchfiles#113 is merged and released (mostly making sure tests don't wait forever if no changes are detected).

Kludex · 2022-04-03T09:44:00Z

I think I'll not have time to review this today, but my initial thoughts are to first deprecate watchgod, and not to remove it directly. As on the user's point of view those are different packages, and it would be a breaking change (even if that package is not supposed to be used in production on uvicorn...).

But... I think it's fine to change all documentation and replace the standard extra requirements. About the tests, I'll need to check.

samuelcolvin · 2022-04-03T09:59:02Z

Makes sense, should be possible to add back the watchgod class, then choose between the three.

samuelcolvin · 2022-04-09T13:18:41Z

AKAIK this is ready to review.

Kludex

I've changed the logic of BaseReload to try and make StatReload and watchfiles share an interface, are you happy with this?

I'm fine with the changes on the BaseReload class. It's easier to understand the logic of run(). Thanks. 👍

I don't care about the change on the reloader_name for each reloader class. Do you @euri10 ?

I've made an initial review. I didn't check the tests yet.

.gitignore

setup.cfg

uvicorn/supervisors/watchgodreload.py

uvicorn/supervisors/basereload.py

uvicorn/supervisors/watchgodreload.py

uvicorn/supervisors/watchfilesreload.py

Kludex · 2022-04-09T13:52:44Z

Thanks for the PR 🙇

Co-authored-by: Marcelo Trylesinski <[email protected]>

samuelcolvin

Suggestions added.

uvicorn/supervisors/basereload.py

uvicorn/supervisors/watchfilesreload.py

uvicorn/supervisors/watchgodreload.py

samuelcolvin · 2022-06-17T16:37:38Z

@Kludex sorry to hassle, why update on this?

Kludex · 2022-06-17T16:43:27Z

Update what?

samuelcolvin · 2022-06-17T16:45:46Z

ty, will check in the following day(s) 🙏

You mentioned you'd check this.

Kludex · 2022-06-17T17:52:05Z

Ah! Sorry!

I'm going to check in some hours. 🙏

uvicorn/supervisors/watchfilesreload.py

tests/supervisors/test_reload.py

Kludex · 2022-06-17T21:02:26Z

@samuelcolvin Done! 👍

Besides those 3 comments, I don't have more to say here. Thanks for the PR! 🙇

Kludex · 2022-06-18T07:32:24Z

I'll see today how's going to be our release schedule (i.e. if there's going to be a patch before merging this). Jfyk

Kludex · 2022-06-18T16:56:36Z

Thanks @samuelcolvin ! 🙇

Sorry for taking so long on the review.

samuelcolvin · 2022-06-18T17:43:14Z

No problem at all, thanks reviewing and all your work.

sovanna · 2022-06-24T11:38:42Z

I needed to install rust in order to install uvicorn to make watchfiles works.

pip install uvicorn[standard] didn't work for me anymore.

I had the issue below.

Collecting watchfiles
  Using cached watchfiles-0.15.0.tar.gz (36 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [6 lines of output]
      Checking for Rust toolchain....

      Cargo, the Rust package manager, is not installed or is not on PATH.
      This package requires Rust and Cargo to compile extensions. Install it through
      the system's package manager or via https://rustup.rs/

      [end of output]

Using inside a Docker container (FROM pypy:3)

samuelcolvin · 2022-06-24T13:25:32Z

Best to create an issue on watchfiles.

Currently we don't distribute binaries for pypy.

You can always skip installing watchfiles and uvicorn will fall back to polling.

bropuffshroom · 2022-07-29T09:06:58Z

I needed to install rust in order to install uvicorn to make watchfiles works.

pip install uvicorn[standard] didn't work for me anymore.

I had the issue below.

Collecting watchfiles
  Using cached watchfiles-0.15.0.tar.gz (36 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [6 lines of output]
      Checking for Rust toolchain....

      Cargo, the Rust package manager, is not installed or is not on PATH.
      This package requires Rust and Cargo to compile extensions. Install it through
      the system's package manager or via https://rustup.rs/

      [end of output]

Using inside a Docker container (FROM pypy:3)

Can confirm this. I ran fastapi in a docker and the reload just didn't work, but older version works.
This basically rendered --reload broken on uvicorn[standard] installation for some environment, without giving any warning or information whatsoever. I just spend a few hours to debug why the auto reload didn't work for me.

I personally think it's probably not a good idea to migrate to watchfiles for now just because watchgod is deprecated.

For my situation (I'm running a linux container on windows), I didn't run into any trouble installing uvicorn[standard] or watchfiles.

For anyone who ran into the same issue, as mentioned above, after the uvicorn[standard] installation, uninstall watchfiles and uvicorn will fall back to StatReload and the reload will be working.

bropuffshroom · 2022-07-29T09:10:18Z

I needed to install rust in order to install uvicorn to make watchfiles works.

pip install uvicorn[standard] didn't work for me anymore.

I had the issue below.

Collecting watchfiles
  Using cached watchfiles-0.15.0.tar.gz (36 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [6 lines of output]
      Checking for Rust toolchain....

      Cargo, the Rust package manager, is not installed or is not on PATH.
      This package requires Rust and Cargo to compile extensions. Install it through
      the system's package manager or via https://rustup.rs/

      [end of output]

Using inside a Docker container (FROM pypy:3)

Can confirm this. I ran fastapi in a docker and the reload just didn't work, but older version works.
This basically rendered --reload broken on uvicorn[standard] installation for some environment, without giving any warning or information whatsoever.

In my very personal opinion I think it's probably not a good idea to migrate to watchfiles for now just because watchgod is deprecated, since I just spend a few hours to debug why the auto reload didn't work for me.

For my situation (I'm running a linux container on windows), I didn't run into any trouble installing uvicorn[standard] or watchfiles.

For anyone who ran into the same issue, as mentioned above, after the uvicorn[standard] installation, uninstall watchfiles and uvicorn will fall back to StatReload and the reload will be working.

samuelcolvin · 2022-07-29T09:15:22Z

Please create new issues rather than commenting on closed PRs. You're issue is not the same as this, both issues have been fixed.

Your issue looks to be the same as samuelcolvin/watchfiles#169, you just need to set WATCHFILES_FORCE_POLLING and reloading will work.

There were many issues with watchgod, hence why I rewrote it to create watchfiles. As the author of both i can assure you watchfiles is much better in many ways.

bropuffshroom · 2022-07-29T09:46:20Z

Please create new issues rather than commenting on closed PRs. You're issue is not the same as this, both issues have been fixed.

Your issue looks to be the same as samuelcolvin/watchfiles#169, you just need to set WATCHFILES_FORCE_POLLING and reloading will help.

There were many issues with watchgod, hence why I rewrote it to create watchfiles. As the author of both i can assure you watchfiles is much better in many ways.

Thanks for your work. Both are excellent libraries.

dvglab · 2022-09-01T10:23:27Z

Please create new issues rather than commenting on closed PRs. You're issue is not the same as this, both issues have been fixed.

Your issue looks to be the same as samuelcolvin/watchfiles#169, you just need to set WATCHFILES_FORCE_POLLING and reloading will work.

There were many issues with watchgod, hence why I rewrote it to create watchfiles. As the author of both i can assure you watchfiles is much better in many ways.

Information about WATCHFILES_FORCE_POLLING is very important. I've lost a lot of hours before found it here. It seems to me it should be on uvicorn site somewhere in documentation. It was a real headache to realize why the reload function don't work on a docker for Windows. Thanks God I found it here.

Kludex · 2022-09-01T10:28:55Z

PR to improve docs are welcome.

* switch from watchgod to watchfiles * fixing ot skipping tests * linting and self.exclude_dirs * move touch_soon * fix tests on windows * fix tests on windows with py37 * add back "WatchGodReload" for backwards compatibility * depreciated message on WatchGodReload * linting * uprev watchfiles and use yield_on_timeout * Apply suggestions from code review Co-authored-by: Marcelo Trylesinski <[email protected]> * suggestions * moving warning call * Move the warning to the WatchGodReload.__init__ * improve coverage * reinstate test_should_detect_new_reload_dirs * linting * Remove main.py * Remove unused code Co-authored-by: Marcelo Trylesinski <[email protected]>

samuelcolvin added 4 commits April 1, 2022 15:01

switch from watchgod to watchfiles

404d423

fixing ot skipping tests

a0e4276

linting and self.exclude_dirs

a287d5c

move touch_soon

aa0f3d2

samuelcolvin marked this pull request as ready for review April 1, 2022 19:36

samuelcolvin added 2 commits April 1, 2022 23:29

fix tests on windows

9fd50d5

fix tests on windows with py37

f500d50

Kludex mentioned this pull request Apr 2, 2022

Add mypy to the pipeline celery/celery#7383

Merged

Kludex added the hold Don't merge yet label Apr 3, 2022

samuelcolvin mentioned this pull request Apr 8, 2022

Test appending to file samuelcolvin/watchfiles#118

Closed

add back "WatchGodReload" for backwards compatibility

2a13f22

samuelcolvin force-pushed the watchfiles branch from ac52b11 to 2a13f22 Compare April 8, 2022 13:55

samuelcolvin added 3 commits April 8, 2022 14:59

depreciated message on WatchGodReload

dc392ba

linting

b749a6c

uprev watchfiles and use yield_on_timeout

c94dfb3

Kludex mentioned this pull request Apr 9, 2022

Add watchfiles based reload class #1419

Closed

Kludex reviewed Apr 9, 2022

View reviewed changes

samuelcolvin and others added 2 commits April 9, 2022 16:40

Apply suggestions from code review

815c16b

Co-authored-by: Marcelo Trylesinski <[email protected]>

suggestions

e69a213

samuelcolvin commented Apr 9, 2022

View reviewed changes

uvicorn/supervisors/basereload.py Show resolved Hide resolved

uvicorn/supervisors/watchfilesreload.py Show resolved Hide resolved

uvicorn/supervisors/watchgodreload.py Show resolved Hide resolved

uvicorn/supervisors/watchgodreload.py Show resolved Hide resolved

moving warning call

3624c3f

samuelcolvin and others added 2 commits June 17, 2022 18:53

Merge branch 'master' into watchfiles

bf027c2

Remove main.py

b93fcae

Kludex reviewed Jun 17, 2022

View reviewed changes

uvicorn/supervisors/watchfilesreload.py Outdated Show resolved Hide resolved

uvicorn/supervisors/watchfilesreload.py Outdated Show resolved Hide resolved

tests/supervisors/test_reload.py Outdated Show resolved Hide resolved

Remove unused code

45e33df

Kludex removed the hold Don't merge yet label Jun 18, 2022

Kludex mentioned this pull request Jun 18, 2022

Version 0.18.0 #1524

Merged

5 tasks

Kludex approved these changes Jun 18, 2022

View reviewed changes

Kludex merged commit b1a4582 into encode:master Jun 18, 2022

browniebroke mentioned this pull request Jul 26, 2022

Switch from watchgod to watchfiles cookiecutter/cookiecutter-django#3791

Merged

samuelcolvin deleted the watchfiles branch September 1, 2022 13:36

danroozemond mentioned this pull request Sep 14, 2022

add note in Settings on using WatchFiles on WSL #1647

Merged

br3ndonland mentioned this pull request Nov 20, 2022

Fix watchfiles file filtering #1770

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch from `watchgod` to `watchfiles` #1437

Switch from `watchgod` to `watchfiles` #1437

samuelcolvin commented Apr 1, 2022 •

edited

Loading

samuelcolvin commented Apr 1, 2022

samuelcolvin commented Apr 2, 2022

Kludex commented Apr 2, 2022 •

edited

Loading

samuelcolvin commented Apr 2, 2022

samuelcolvin commented Apr 3, 2022

Kludex commented Apr 3, 2022 •

edited

Loading

samuelcolvin commented Apr 3, 2022

samuelcolvin commented Apr 9, 2022

Kludex left a comment

Kludex commented Apr 9, 2022

samuelcolvin left a comment

samuelcolvin commented Jun 17, 2022

Kludex commented Jun 17, 2022

samuelcolvin commented Jun 17, 2022

Kludex commented Jun 17, 2022

Kludex commented Jun 17, 2022

Kludex commented Jun 18, 2022

Kludex commented Jun 18, 2022

samuelcolvin commented Jun 18, 2022

sovanna commented Jun 24, 2022

samuelcolvin commented Jun 24, 2022

bropuffshroom commented Jul 29, 2022

bropuffshroom commented Jul 29, 2022

samuelcolvin commented Jul 29, 2022 •

edited

Loading

bropuffshroom commented Jul 29, 2022

dvglab commented Sep 1, 2022

Kludex commented Sep 1, 2022

Switch from watchgod to watchfiles #1437

Switch from watchgod to watchfiles #1437

Conversation

samuelcolvin commented Apr 1, 2022 • edited Loading

samuelcolvin commented Apr 1, 2022

samuelcolvin commented Apr 2, 2022

Kludex commented Apr 2, 2022 • edited Loading

samuelcolvin commented Apr 2, 2022

samuelcolvin commented Apr 3, 2022

Kludex commented Apr 3, 2022 • edited Loading

samuelcolvin commented Apr 3, 2022

samuelcolvin commented Apr 9, 2022

Kludex left a comment

Choose a reason for hiding this comment

Kludex commented Apr 9, 2022

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin commented Jun 17, 2022

Kludex commented Jun 17, 2022

samuelcolvin commented Jun 17, 2022

Kludex commented Jun 17, 2022

Kludex commented Jun 17, 2022

Kludex commented Jun 18, 2022

Kludex commented Jun 18, 2022

samuelcolvin commented Jun 18, 2022

sovanna commented Jun 24, 2022

samuelcolvin commented Jun 24, 2022

bropuffshroom commented Jul 29, 2022

bropuffshroom commented Jul 29, 2022

samuelcolvin commented Jul 29, 2022 • edited Loading

bropuffshroom commented Jul 29, 2022

dvglab commented Sep 1, 2022

Kludex commented Sep 1, 2022

Switch from `watchgod` to `watchfiles` #1437

Switch from `watchgod` to `watchfiles` #1437

samuelcolvin commented Apr 1, 2022 •

edited

Loading

Kludex commented Apr 2, 2022 •

edited

Loading

Kludex commented Apr 3, 2022 •

edited

Loading

samuelcolvin commented Jul 29, 2022 •

edited

Loading