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

Propagate superficial parameter to remove_watch_with_id #57

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

xaf
Copy link
Contributor

@xaf xaf commented Jul 29, 2018

The remove_watch method takes a superficial parameter but never used
it, which made the remove_watch_with_id function set it as False by default.
This fixes that behavior by propagating the parameter.

Signed-off-by: Raphaël Beamonte [email protected]

The remove_watch method takes a superficial parameter but never used
it, which made the remove_watch_with_id function set it as False by default.
This fixes that behavior by propagating the parameter.

Signed-off-by: Raphaël Beamonte <[email protected]>
@xaf
Copy link
Contributor Author

xaf commented Jul 29, 2018

Please note that failing tests are not related...

@Elias481
Copy link

Sure it's related. Otherwise You would not get that error on Your commit normally...

It's not that You did the wrong thing, but another bug has to be fixed if You fix this bug: the superficial argument must be False for the IN_MOVED_FROM handling. A moved directory is not automatically deregistered from inotify.
But to be on the safest side the case that the diretory is deleted immediately after rename/remove but before the unregister took place some additional work would be needed to not throw an error then as the delete would have already removed the watch in that case.. I assume this is a rare case, but worth noting and/or to think about.
Simple solution: catch exception resulting from library call an ignore - as the library call occurs after the superficial actions the directory is already unregistered from adaptor.

Also note: as You submitted #58 separatly and after this, for this to work You would need to patch this for the correct block and also for the block that should be used for IN_DELETE normally, an patch this back in #58 to re-enable superficial for the IN_DELETE case.

@dsoprea dsoprea merged commit 7202fc4 into dsoprea:master Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants