Remove pyinotify dependency

[enykeev]

Resolves #3934

[Kami]

What's our plan for this?

Just removing this dependency without any other changes will break linux pack (file watch sensor) out of the box, so we shouldn't do that, imo.

[Kami]

If we want to remove it "correctly", we need to update installer scripts / package post install script to correctly create virtual environment for those packages (IIRC, we don't do that right now).

We have also been going back and forth on this for a while now - at first, core and linux packages were not part of standard installation package and they were installed from Exchange, now they are.

In theory, it shouldn't matter how those core / default packages are installed, as long as it's transparent to the end user and packages are always installed no matter what installation method user uses.

In short - I am for removing this dependency, but only if and when we found better way to mange installation of it (e.g. virtualenv creation for {core,linux} in post install or similar).

/cc @LindsayHill

[enykeev]

Just removing this dependency without any other changes will break linux pack (file watch sensor) out of the box, so we shouldn't do that, imo.

I should have pointed out that the only reason for this PR was to check if the change triggers something in our CI process.

we need to update installer scripts / package post install script to correctly create virtual environment for those packages

yep, we need to. I don't really see why core or linux packs should be any different from other packs.

[enykeev]

we need to update installer scripts / package post install script to correctly create virtual environment for those packages

Actually, in light of recent discussion in regards to the future of pack management, I'd say that core and linux pack should be also installed via pack management routine i.e. via API. This would ensure that venv of all the packs is in order and will allow a user to depend on a particular version of core pack if the need arise.

[arm4b]

In short - I am for removing this dependency, but only if and when we found better way to mange installation of it (e.g. virtualenv creation for {core,linux} in post install or similar).
package post install script to correctly create virtual environment for those packages (IIRC, we don't do that right now)

Running st2 pack install ... during the postinst will bring a regression in deb/rpm installation itself.
Adding an external dependency pulling something dynamic like that and doing compilation on users's machine post deb/rpm install is a bad practice. Instead of that, all the external resources should be prepared during the package build stage. That's why we went from old deb/rpm and doing post-processing like pip install on users's machine to deb/rpm with dh_virtualenv with pre-installed pip dependencies and everything bundled on the build stage.
Additionally, installing a package is not idempotent and potentially could fail in any step, see #3052. Also see one of the examples StackStorm/st2-packages#453 why doing something like that in postinst is a bad practice.

yep, we need to. I don't really see why core or linux packs should be any different from other packs.

Disagree.
Having core packs located/shipped externally will add additional installation step and so complicate the story and UX.
Ideally if user has the "core" StackStorm system ready to work right after installing deb/rpm packages.

Add here air-gapped installation requirements, where users don't have immediate access to StackStorm exchange-like resources.

In short, the easy check is if it's done right: installing deb/rpm shouldn't rely on any external resource, except of deb/rpm-like dependencies and shouldn't perform any type of compilation/potentially long execution, otherwise we'll return back to era with installation issues StackStorm had before.

(BTW think about our friends who ship a product that has pack install in postinst which requires 6GB of memory for pip compilation, - that's a classic example of doing it wrong in product deployment design).

[m4dcoder]

@enykeev @armab @Kami Can you guys resolve your differences here?

[blag]

Superceded by #4145.

[arm4b]

👍

[blag]

And #4145 was superseded by #4191.