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

[BSD] Restore support #641

Merged
merged 3 commits into from
Feb 7, 2020
Merged

Conversation

evilham
Copy link
Contributor

@evilham evilham commented Feb 7, 2020

See: https://cirrus-ci.com/task/5908114574671872
Fixes #637

For some reason GH is not taking into account that some commits are merged already? Maybe some cache issue...

@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 7, 2020

You need to rebase your branch on master before adding the new commit.

Changes in kqueue observer:
- Some refactoring to have the data flow be clearer
- Conform better to what the test suite expects
- Be a bit more correct
src/watchdog/observers/kqueue.py Outdated Show resolved Hide resolved
src/watchdog/observers/kqueue.py Outdated Show resolved Hide resolved
src/watchdog/observers/kqueue.py Outdated Show resolved Hide resolved
src/watchdog/observers/kqueue.py Show resolved Hide resolved
@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 7, 2020

This is awesome, many thanks @evilham !!

@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 7, 2020

In the Cirrus-CI report, there are those lines at the end:

Exception in thread Thread-8:
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/threading.py", line 926, in _bootstrap_inner
    self.run()
  File "/tmp/cirrus-ci-build/src/watchdog/observers/api.py", line 145, in run
    self.queue_events(self.timeout)
  File "/tmp/cirrus-ci-build/src/watchdog/observers/kqueue.py", line 668, in queue_events
    self.watch.is_recursive)
  File "/tmp/cirrus-ci-build/src/watchdog/utils/dirsnapshot.py", line 253, in __init__
    st = self.stat(path)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmp_tebxtss/dir1'

Does that mean the test was retried and passes?

@evilham
Copy link
Contributor Author

evilham commented Feb 7, 2020

Thanks for the comment, totally missed that :-).

Re: message, yeah, I think that test is marked as flaky and it passes on a second go; didn't observe that locally.

It'd look like the directory gets removed before a new dir snapshot was created (L668); unsure if this is an error in kqueue or in dirsnapshot o.ô.
What would you think is best to handle this case?

Waiting for https://cirrus-ci.com/task/5449374754930688 to finish and check if it's flaky as well.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 7, 2020

If this is a random error, let's do nothing :)

@evilham
Copy link
Contributor Author

evilham commented Feb 7, 2020

Yup, cirrus-ci didn't replicate that either:
https://cirrus-ci.com/task/5274372285923328

@BoboTiG BoboTiG merged commit 6e01c8d into gorakhargosh:master Feb 7, 2020
@BoboTiG
Copy link
Collaborator

BoboTiG commented Feb 7, 2020

Merged, thank you for your time 🍾

@evilham
Copy link
Contributor Author

evilham commented Feb 7, 2020

Party! thank you for your kind assistance :-).

CCP-Aporia pushed a commit to CCP-Aporia/watchdog that referenced this pull request Aug 13, 2020
* [BSD] Pass tests \o/

Changes in kqueue observer:
- Some refactoring to have the data flow be clearer
- Conform better to what the test suite expects
- Be a bit more correct

* [BSD] Restore support (changelog)

Fixes gorakhargosh#637

* [BSD] Restore support (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BSD] Pass tests with KqueueEmitter
2 participants