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 support for non-recursive watches in FSEventsEmitter #779

Conversation

CCP-Aporia
Copy link
Contributor

The underlying fseventsd service always observes recursively. As such we're adding a filter in FSEventEmitter to support the non-recursive behaviour that is supported by other emitters.

Fixes #771

The underlying `fseventsd` service always observes recursively. As such we're adding a filter in `FSEventEmitter` to support the non-recursive behaviour that is supported by other emitters.
Copy link
Collaborator

@BoboTiG BoboTiG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

Could you add the changelog entry 🙏 ?

src/watchdog/observers/fsevents.py Show resolved Hide resolved
tests/test_emitter.py Show resolved Hide resolved
@CCP-Aporia
Copy link
Contributor Author

Of course. Was just waiting for the review before adding the relevant changelog entry. :-) Won't get around to all of it before Monday, though.

Test a few more scenarios for non-recursive watches. Do note that at least on macOS we'll get a `DirModifiedEvent` for the created subdirectory.
@CCP-Aporia
Copy link
Contributor Author

Turns out that the additional test cases fail on Ubuntu and Windows. What would be the expected behaviour on these platforms for the added scenarios? The same as on macOS?

We're fixing a defect on macOS after all, and addressing the failures on other platforms looks like opening a can of worms that is not related to the original issue.
@BoboTiG
Copy link
Collaborator

BoboTiG commented Apr 28, 2021

You are right with the last commit, let's tackle macOS only for now :)

@CCP-Aporia
Copy link
Contributor Author

CCP-Aporia commented May 3, 2021

@BoboTiG - shall we get this merged? 😄 The failed test on macOS / 3.7 seems to be a timing issue in a flaky test

@BoboTiG
Copy link
Collaborator

BoboTiG commented May 3, 2021

Oh yes, sorry for the delay!

Thanks for the patch :)

@BoboTiG BoboTiG merged commit 1c67997 into gorakhargosh:master May 3, 2021
@oprypin
Copy link
Contributor

oprypin commented May 17, 2021

Just making sure you're aware, this pull request caused issue #797.
At least in some cases it seems like non-recursive watches on macOS produce no events at all.

copybara-service bot pushed a commit to google/pigweed that referenced this pull request Jun 10, 2021
See gorakhargosh/watchdog#779

No-Docs-Update-Reason: bug fix
Change-Id: I6a5fd6bc7964e2da672593d50180cb9aa7b6b8c1
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/48521
Reviewed-by: Max Koopman <koopman@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Michael Spang <spang@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mac] FSEvents handler ignores the 'recursive' parameter
3 participants