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

feat: support for 'create' event from the inotify watcher #11

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

neelayu
Copy link

@neelayu neelayu commented Nov 21, 2022

Inotify watcher type will now emit "CREATE" file event when the watcher is defined on the parent directory.

PS: It also shows commit from the Previous unmerged PR

@neelayu
Copy link
Author

neelayu commented Dec 6, 2022

@powersj Can you go through this PR?

Copy link

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Here is my hesitation: this fork has not been updated in years and I'm not sure what to think about the change in behavior. I think it looks good in general, @srebhan thoughts?

@neelayu
Copy link
Author

neelayu commented Dec 9, 2022

Makes sense. Lets build a consensus on this and then we can proceed.

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks good to me. However, as @powersj said, it might make sense to completely switch the underlying library in Telegraf, e.g. to https://github.com/fsnotify/fsnotify...

@neelayu
Copy link
Author

neelayu commented Jan 4, 2023

@srebhan that's a reasonable ask. but could involve a lot of work since this repo also uses fsnotify for inotify watch type. poll is not supported by fsnotify so might have to think about other alternatives

@srebhan srebhan assigned srebhan and powersj and unassigned srebhan Jan 10, 2023
@srebhan
Copy link
Member

srebhan commented Jan 10, 2023

I agree but I would also welcome effort in that direction. https://pkg.go.dev/github.com/docker/docker/pkg/filenotify is a wrapper around fsnotify with support for polling. Didn't test that yet nor looked too deep into it. Maybe you can take a look?

@neelayu
Copy link
Author

neelayu commented Apr 6, 2023

Hey @srebhan I looked at the above pkg and it seems they have removed in from the source. Although, we can still import it, but there is no active maintenance. Can we proceed with the current state?

@srebhan
Copy link
Member

srebhan commented Apr 14, 2023

@redbaron
Copy link

Another more maintained candidate is https://pkg.go.dev/kubesphere.io/fluentbit-operator/pkg/filenotify

@powersj powersj merged commit 3c9d721 into influxdata:master Jul 19, 2024
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.

4 participants