samuelcolvin / watchfiles

Simple, modern and fast file watching and code reload in Python.
https://watchfiles.helpmanual.io
MIT License
1.75k stars 108 forks source link

Ignore error_handler errors under ignore_permission_denied = True? #266

Closed earshinov closed 1 week ago

earshinov commented 7 months ago

Description

Hey!

For local development, we are running our Django app inside a Linux Docker container, launched from Windows host with help of WSL2, with sources typically mounted from the Windows file system. We are using watchfiles instead of Django's built-in reloader with help of django_watchfiles. Thanks to this code, watchfiles understands that it is launched from a Windows host and falls back to PollWatcher (👌).

We are facing an issue when watchfiles hits an exception and exits (also bringing down the Django app and the Docker container) when we launch mypy in the same running container:

_rust_notify.WatchfilesRustInternalError: error in underlying watcher: IO error for operation on /opt/project/.mypy_cache/3.10/iniconfig/__init__.meta.json.894e30fd1ecd2163: No such file or directory (os error 2) about ["/opt/project/.mypy_cache/3.10/iniconfig/__init__.meta.json.894e30fd1ecd2163"]

I just filed a bug report to notify (https://github.com/notify-rs/notify/issues/581), but I don't think that ignoring errors inside notify are a good idea. It might simply be inappropriate in some use cases which inotify has to support. However, ignoring errors here in watchfiles sounds reasonable, particularly because some errors are already being ignored, see !224.

I am no expert in Rust, but I assume that the code added in !224 only handles errors reported immediately when .watch() is called. And errors reported through event_handler are propagated as before, with PyResult::Err converted into _rust_notify.WatchfilesRustInternalError Python exception somewhere here, see the error message above.

I think it would be reasonable to ignore errors here when ignore_permission_denied = True.

For the moment, I am not ready to tinker with Rust, so I made a PR into django-watchfiles to circumvent this on the outside. But really, I think that this code should be unnecessary and the errors should be ignored right in watchfiles.

Example Code

No response

Watchfiles Output

No response

Operating System & Architecture

Host: Windows 11 x64 Version 23H2 (0S Build 22631.3296)

wsl cat /proc/version:

Linux version 5.10.102.1-microsoft-standard-WSL2 (oe-user@oe-host) (x86_64-msft-linux-gcc (GCC) 9.3.0, GNU ld (GNU Binutils) 2.34.0.20200220) #1 SMP Wed Mar 2 00:30:59 UTC 2022

Docker container: python:3.11-bullseye-slim

python -c 'import platform; print(platform.platform()); print(platform.version())':

Linux-5.10.102.1-microsoft-standard-WSL2-x86_64-with-glibc2.31
#1 SMP Wed Mar 2 00:30:59 UTC 2022

Environment

Windows host+ Docker container running in WSL, see full description of the setup above

Python & Watchfiles Version

python: 3.11.3 (tags/v3.11.3:f3909b8, Apr 4 2023, 23:49:59) [MSC v.1934 64 bit (AMD64)], watchfiles: 0.21.0

Rust & Cargo Version

No response

earshinov commented 7 months ago

All references at a glance:

earshinov commented 1 week ago

Seems to be unnecessary after https://github.com/samuelcolvin/watchfiles/pull/301