Recommend dropping pyinotify as a dependency

[tonybaloney]

st2 has a dependency on pyinotify, which is poorly maintained. Digging into it, it looks like no code in st2 references it, only the linux pack, which is no longer part of this code base.

Recommend moving to another, dropping it completely or using https://github.com/chrisjbillington/inotify_simple

pyinotify is also a requirement in @Kami's custom build of logshipper git+https://github.com/Kami/logshipper.git@stackstorm_patched#egg=logshipper

[LindsayHill]

only the linux pack, which is no longer part of this code base.

Linux pack is still sorta kinda part of this code base (https://github.com/StackStorm/st2/tree/master/contrib/linux). But I guess it has pynotify in its virtualenv, so it doesn't need to be in 'core'.

As an aside, this would make my life a little easier when doing st2docs work on Mac.

[tonybaloney]

and logshipper is only a dependency because of the file_watch_sensor in the contrib/linux/sensors directory. Are there any plans to move that out of this core repo?

[tonybaloney]

@LindsayHill you read my mind :-) it would make my life easier to be able to run the unit tests on my Mac and not have to sync the repo with a server for minor changes. This is the only dep that I could find

[LindsayHill]

Are there any plans to move that out of this core repo?

Funnily enough that came up earlier today. No immediate plans to do so, but it probably should get done. I still want the core ST2 RPMs/Debs to include the Linux pack, but I think it should probably live in its own pack on exchange.stackstorm.org. Perhaps include it as a subtree?

[LindsayHill]

@Kami what's your thoughts about removing pyinotify from st2 requirements? I think it's a good idea. As an aside, I'm also wondering if we should switch to the pypi version of logshipper, since it now includes your changes. May be that we don't require logshipper in core st2 requirements either, only in the Linux pack.

[Kami]

@LindsayHill Yeah, I wish we could switch to using official / upstream version, but not all the changes we use have been merged yet (https://github.com/ondergetekende/logshipper/pull/11) and even though one of the PRs has been merged, new version hasn't been published to PyPi so we would still need to use version from git...

[Kami]

As far as actual pyinotify issue goes - what's the actual root problem?

Reliance on C extension and not supporting Python 3? If so, and the other inotify wrapper handles that better, we would need to modify logshipper to use that library.

[blag]

The issue with relying on pyinotify is (in decreasing order of severity):

1. It refuses to even install on OS X. I understand it not working on OS X (because inotify is Linux-only), but it's got this in its setup.py:

   if not platform.startswith('linux') and not platform.startswith('freebsd'):
       sys.stderr.write("inotify is not available on %s\n" % platform)
       sys.exit(1)

   which prevents it from even being installed on OS X (and therefore prevents ST2 from being "successfully" installed via pip)

2. It's not maintained anymore - the author hasn't done anything (no code, no issue responses/closes, no PRs - zero activity) since 2015. These PRs: seb-m/pyinotify#38, seb-m/pyinotify#63, seb-m/pyinotify#139, seb-m/pyinotify#153 are all stacked up and look ready to merge, but there's been zero movement or response from the author.
3. I emailed the author directly a few days ago inquiring if he is still interested in maintaining it and haven't gotten a response. Their blog hasn't been updated since 2014.

I think at this point we either need to remove it as a dependency of ST2 itself (should be fine as a dependency of the Linux contrib/packs though), or to fork it and start merging in PRs and try to maintain a fork of it ourselves. I'm not super happy about maintaining a fork, but ¯\_(ツ)_/¯. I suspect other people in the community would appreciate somebody forking it even if they only merge in existing PRs.

@Kami Let me know if you're okay with just dropping it or if you'd like me to fork it. I think I can take on maintaining it.

[Kami]

@blag I'm fine with dropping it, but we need to find a solution for the file watch sensor in linux pack. Linux pack is included / bundled with the default StackStorm distribution so if we simply drop that dependency it will break file watch sensor (it won't work out of the box anymore).

One solution is to move to a different library for file watching which is cross operating system compatible, but that means we will also need to update our logshipper fork so it uses that library, or ideally, we can find some other maintained replacement for logshipper which doesn't rely on pyinotify.

[arm4b]

The proposed solution by @Kami sounds good to me too, keeping in mind concerns described in https://github.com/StackStorm/st2/pull/3942#issuecomment-360475286

[blag]

A different PyInotify project looks promising - maybe we can migrate logshipper to that instead? That library also doesn't have anything in its setup.py that prevents it from being installed on OS X.

[bigmstone]

I'd be fine w/ removing/replacing the logshipper dep too. It brings in nice nuggets like the statsd collision I hit in the metrics PR.

[blag]

I have created PR #4191 - implement our own tailing function based on the one from logshipper.

My version uses the maintained PyInotify package (inotify on PyPI) instead.

Please take a look at that and let me know what you think.