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 ignored_dirs param to InotifyTree(s) #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Larivact
Copy link

You should be able to ignore specific directories.

@dsoprea dsoprea force-pushed the master branch 7 times, most recently from 1f28aec to 01bfa05 Compare December 1, 2017 17:44
@@ -227,53 +227,55 @@ def event_gen(self):

class InotifyTree(BaseTree):
def __init__(self, path, mask=inotify.constants.IN_ALL_EVENTS,
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S):
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S, ignored_dirs=[]):
Copy link

@blag blag May 29, 2018

Choose a reason for hiding this comment

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

You probably don't want to use a mutable object for the default value of an argument. See here for more information.

Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't matter. We're not changing anything.



class InotifyTrees(BaseTree):

def __init__(self, paths, mask=inotify.constants.IN_ALL_EVENTS,
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S):
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S, ignored_dirs=[]):
Copy link

Choose a reason for hiding this comment

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

@dsoprea
Copy link
Owner

dsoprea commented Jul 7, 2018

This is useful. Please fix the conflicts and account for the refactor to os.walk, and we'll get it merged.

@XaF
Copy link
Contributor

XaF commented Jul 29, 2018

You probably should prefer using an ignore function instead of an ignore list, as shutil.copytree does.

@Larivact
Copy link
Author

@dsoprea What "refactor to os.walk"? Do you mean #48? How am I supposed to account for an unfinished pull request?

Copy link

@Elias481 Elias481 left a comment

Choose a reason for hiding this comment

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

The add_watch called from (_)BaseTree's event_gen should also ignore the configured directories (for consitency).
But anyway, should not be done on outdated base...

@@ -227,53 +227,55 @@ def event_gen(self):

class InotifyTree(BaseTree):
def __init__(self, path, mask=inotify.constants.IN_ALL_EVENTS,
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S):
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S, ignored_dirs=[]):
super(InotifyTree, self).__init__(mask=mask, block_duration_s=block_duration_s)

Choose a reason for hiding this comment

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

You should push ignored_dirs down to (_)BaseTree's init and store it on object from there as it needs to be referenced there (line 216).



class InotifyTrees(BaseTree):

def __init__(self, paths, mask=inotify.constants.IN_ALL_EVENTS,
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S):
block_duration_s=_DEFAULT_EPOLL_BLOCK_DURATION_S, ignored_dirs=[]):
super(InotifyTrees, self).__init__(mask=mask, block_duration_s=block_duration_s)

Choose a reason for hiding this comment

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

You should push ignored_dirs down to (_)BaseTree's init and store it on object from there as it needs to be referenced there (line 216).

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.

5 participants