Skip to content

Commit

Permalink
Better error on OS watcher limit (#208)
Browse files Browse the repository at this point in the history
* better error on OS watcher limit, #207

* skip message check on other OSes

* fix linting

* more error details, skip build on PR

* fix docs build, support rust 1.56

* file not found on force polling

* fix error typo
  • Loading branch information
samuelcolvin authored Nov 11, 2022
1 parent 94ef903 commit eb8771c
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 39 deletions.
16 changes: 8 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
- '3.8'
- '3.9'
- '3.10'
- '3.11-dev'
- '3.11'
- 'pypy3.8'
- 'pypy3.9'

Expand Down Expand Up @@ -87,7 +87,7 @@ jobs:

- run: pip install -r requirements/pyproject.txt -r requirements/linting.txt

- run: pip install .
- run: pip install -e .

- uses: pre-commit/[email protected]
with:
Expand Down Expand Up @@ -119,6 +119,7 @@ jobs:
name: >
build on ${{ matrix.platform || matrix.os }} (${{ matrix.target }} - ${{ matrix.manylinux || 'auto' }})
if: "startsWith(github.ref, 'refs/tags/') || github.ref == 'refs/heads/main' || contains(github.event.pull_request.labels.*.name, 'Full Build')"
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -173,8 +174,6 @@ jobs:
run: cargo update -p watchfiles_rust_notify
if: "startsWith(github.ref, 'refs/tags/')"

- run: pip install twine

- name: build sdist
if: ${{ matrix.os == 'ubuntu' && matrix.target == 'x86_64' && matrix.manylinux == 'auto' }}
uses: messense/maturin-action@v1
Expand All @@ -199,8 +198,6 @@ jobs:

- run: ${{ matrix.ls || 'ls -lh' }} dist/

- run: twine check dist/*

- uses: actions/upload-artifact@v3
with:
name: pypi_files
Expand Down Expand Up @@ -235,10 +232,13 @@ jobs:
ls dist/*cp37-abi3-manylinux*x86_64.whl | head -n 1
python -m zipfile --list `ls dist/*cp37-abi3-manylinux*x86_64.whl | head -n 1`
- run: pip install twine
- run: twine check dist/*

# Used for branch protection checks, see https://github.com/marketplace/actions/alls-green#why
check:
if: always()
needs: [test, lint, docs, build, list-pypi-files]
needs: [test, lint, docs]
runs-on: ubuntu-latest
steps:
- name: Decide whether the needed jobs succeeded or failed
Expand All @@ -247,7 +247,7 @@ jobs:
jobs: ${{ toJSON(needs) }}

release:
needs: [build, docs]
needs: [build, check, docs]
if: "success() && startsWith(github.ref, 'refs/tags/')"
runs-on: ubuntu-latest

Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ testcov: test

.PHONY: docs
docs:
rm -f watchfiles/*.so
mkdocs build

.PHONY: all
Expand Down
27 changes: 3 additions & 24 deletions docs/plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,14 @@
from mkdocs.structure.files import Files
from mkdocs.structure.pages import Page

try:
import pytest
except ImportError:
pytest = None

logger = logging.getLogger('mkdocs.test_examples')
logger = logging.getLogger('mkdocs.plugin')


def on_pre_build(config: Config):
test_examples()


def test_examples():
"""
Run the examples tests.
Not doing anything here anymore.
"""
if not pytest:
logger.info('pytest not installed, skipping examples tests')
else:
logger.info('running examples tests...')
try:
pytest.main(['--version'])
except AttributeError:
# happens if pytest is not properly installed
logger.info('pytest not installed correctly, skipping examples tests')
else:
return_code = pytest.main(['-q', '-p', 'no:sugar', 'tests/test_docs.py'])
if return_code != 0:
logger.warning('examples tests failed')
pass


def on_files(files: Files, config: Config) -> Files:
Expand Down
35 changes: 29 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@ extern crate notify;
extern crate pyo3;

use std::collections::HashSet;
use std::io::ErrorKind as IOErrorKind;
use std::path::Path;
use std::sync::{Arc, Mutex};
use std::thread::sleep;
use std::time::{Duration, SystemTime};

use pyo3::create_exception;
use pyo3::exceptions::{PyFileNotFoundError, PyRuntimeError, PyTypeError};
use pyo3::exceptions::{PyFileNotFoundError, PyOSError, PyPermissionError, PyRuntimeError, PyTypeError};
use pyo3::prelude::*;

use notify::event::{Event, EventKind, ModifyKind, RenameMode};
use notify::{
Config as NotifyConfig, ErrorKind, PollWatcher, RecommendedWatcher, RecursiveMode, Result as NotifyResult, Watcher,
Config as NotifyConfig, ErrorKind as NotifyErrorKind, PollWatcher, RecommendedWatcher, RecursiveMode,
Result as NotifyResult, Watcher,
};

create_exception!(
Expand Down Expand Up @@ -43,6 +45,26 @@ struct RustNotify {
watcher: WatcherEnum,
}

fn map_watch_error(error: notify::Error) -> PyErr {
let err_string = error.to_string();
match error.kind {
NotifyErrorKind::PathNotFound => return PyFileNotFoundError::new_err(err_string),
NotifyErrorKind::Generic(ref err) => {
// on Windows, we get a Generic with this message when the path does not exist
if err.as_str() == "Input watch path is neither a file nor a directory." {
return PyFileNotFoundError::new_err(err_string);
}
}
NotifyErrorKind::Io(ref io_error) => match io_error.kind() {
IOErrorKind::NotFound => return PyFileNotFoundError::new_err(err_string),
IOErrorKind::PermissionDenied => return PyPermissionError::new_err(err_string),
_ => (),
},
_ => (),
};
PyOSError::new_err(format!("{} ({:?})", err_string, error))
}

// macro to avoid duplicated code below
macro_rules! watcher_paths {
($watcher:ident, $paths:ident, $debug:ident, $recursive:ident) => {
Expand All @@ -52,9 +74,7 @@ macro_rules! watcher_paths {
RecursiveMode::NonRecursive
};
for watch_path in $paths.into_iter() {
$watcher
.watch(Path::new(&watch_path), mode)
.map_err(|e| PyFileNotFoundError::new_err(format!("{}", e)))?;
$watcher.watch(Path::new(&watch_path), mode).map_err(map_watch_error)?;
}
if $debug {
eprintln!("watcher: {:?}", $watcher);
Expand Down Expand Up @@ -145,6 +165,9 @@ impl RustNotify {
};
macro_rules! create_poll_watcher {
($msg_template:literal) => {{
if watch_paths.iter().any(|p| !Path::new(p).exists()) {
return Err(PyFileNotFoundError::new_err("No such file or directory"));
}
let delay = Duration::from_millis(poll_delay_ms);
let config = NotifyConfig::default().with_poll_interval(delay);
let mut watcher = match PollWatcher::new(event_handler, config) {
Expand All @@ -167,7 +190,7 @@ impl RustNotify {
}
Err(error) => {
match &error.kind {
ErrorKind::Io(io_error) => {
NotifyErrorKind::Io(io_error) => {
if io_error.raw_os_error() == Some(38) {
// see https://github.com/samuelcolvin/watchfiles/issues/167
// we callback to PollWatcher
Expand Down
Empty file added tests/__init__.py
Empty file.
15 changes: 14 additions & 1 deletion tests/test_rust_notify.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from watchfiles._rust_notify import RustNotify

skip_unless_linux = pytest.mark.skipif('linux' not in sys.platform, reason='avoid time differences on other platforms')
skip_unless_linux = pytest.mark.skipif('linux' not in sys.platform, reason='avoid differences on other systems')
skip_windows = pytest.mark.skipif(sys.platform == 'win32', reason='fails on Windows')


Expand Down Expand Up @@ -151,6 +151,19 @@ def test_does_not_exist(tmp_path: Path):
RustNotify([str(p)], False, False, 0, True)


@skip_unless_linux
def test_does_not_exist_message(tmp_path: Path):
p = tmp_path / 'missing'
with pytest.raises(FileNotFoundError, match='No such file or directory'):
RustNotify([str(p)], False, False, 0, True)


def test_does_not_exist_polling(tmp_path: Path):
p = tmp_path / 'missing'
with pytest.raises(FileNotFoundError, match='No such file or directory'):
RustNotify([str(p)], False, True, 0, True)


def test_rename(test_dir: Path):
watcher = RustNotify([str(test_dir)], False, False, 0, True)

Expand Down

0 comments on commit eb8771c

Please sign in to comment.