Directory name changes are not managed#1
Directory name changes are not managed#1dfranganillo wants to merge 11 commits into
Conversation
|
Thanks for the contribution! I'll definitely be taking a look at this after work. @bos has an upstream repository at https://bitbucket.org/bos/python-inotify/ (though it's in mercurial). I actually made this so my coworkers could easily use it as we're a git shop. If he would prefer to run the mirror on github I'd be glad to swap this repository over to him 😀 |
|
Wow, I really searched for the original repo of the original author @bos , sorry I didn't found it!! Anyway, I cannot find any differences between your repo and the original from bos so being it an almost forgotten repo we could continue from this point onwards. |
|
Just a little update! I've hooked up nosetests to our |
if add_watch fails wd==-1, test it
Added version number. Return a value instead of a function whenever we read the procfs values of inotify
EOL and spaces havok
|
I've pushed some more commits, one of them is quite critical as it solves a big memory leak. |
|
Hey, thanks for the update. I'll review them when I get a chance. Sorry for not getting on your earlier changes - I've been quite busy! |
|
No problem! |
There was a problem hiding this comment.
Which exceptions are being caught here? We usually try to avoid blanket exceptions like this.
There was a problem hiding this comment.
I've removed that ugly exception :/
There was a problem hiding this comment.
See above comment about exception. We generally avoid them because they can mask deeper issues in the code. If you want you can catch a specific instance of an exception class I'd be 100% onboard.
There was a problem hiding this comment.
Yeah, this one was trying to ignore _remove exceptions and I forgot to
remove the handler.
2014-03-28 16:47 GMT+01:00 Matthew Thompson notifications@github.com:
In inotify/watcher.py:
@@ -177,11 +192,15 @@ def read(self, bufsize=None):
events = [] for evt in inotify.read(self.fd, bufsize):
events.append(Event(evt, self._wds[evt.wd][0]))if evt.mask & inotify.IN_IGNORED:self._remove(evt.wd)elif evt.mask & inotify.IN_UNMOUNT:self.close()try:if evt.mask & inotify.IN_IGNORED:self._remove(evt.wd)elif evt.mask & inotify.IN_UNMOUNT:self.close()else:events.append(Event(evt, self._wds[evt.wd][0]))except:See above comment about exception. We generally avoid them because they
can mask deeper issues in the code. If you want you can catch a specific
instance of an exception class I'd be 100% onboard.Reply to this email directly or view it on GitHubhttps://github.com//pull/1/files#r11071646
.
Daniel Franganillo Corrales
|
Sorry if it seems like I'm nitpicking - we use this for a fairly important piece of our infrastructure so I'd want to make sure it's up to our internal standards before merging. 😄 |
|
No problem, there's always room to learn something 👍 |
|
I was wondering if there's any chance that it'll be merged someday. |
Whenever a directory name changes, even though the wd is the same, _wds and _paths directories are not updated.