Fix path to st2 virtual environment #571
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
enykeev
approved these changes
Jun 28, 2018
arm4b
suggested changes
Jun 28, 2018
Member
There was a problem hiding this comment.
@bigmstone @m4dcoder @Kami @enykeev
We shouldn't run any long-running, downloading, installing or compiling executions in deb/rpm post-install scriptlets (eg. when user installs the package).
Instead all the python dependencies and stuff should be bundled during the package build stage.
That's the main point of this dh_virtualenv packaging work we did before: off-load the installation executions on build time, not on run-time or install-time.
Please find the way to run install on build-time and bundle changes in virtualenv.
Member
|
To be clear, this is what happens on st2 package install time, while processing like that should be done on deb/rpm build time and already bundled in a shipped package: |
Member
Author
|
@armab I agree (as far as a long term solution goes). We had a discussion on this on Slack and here - StackStorm/st2#4210 (comment) I would prefer to do it the right way, but we are already a bit late with v2.8.0 and doing it correct way is a lot more involved. We started preparing the foundation for runners as a packages in the past, but we weren't in a rush to move and ship runners as packages (build wheels and handle them as any other Python dependencies). Everything worked just fine as it was, so there was no rush, but Orchestra uses stevedore to dynamically load filter plugins (good thing, imo) and that won't work with the current approach. We either need to do a workaround similar to one here for v2.8.0 or do it correctly. I'm we decide to go with "do it correctly for v2.8.0", I'm sure that will delay the release for at least 3 days and it's also not something I want to rush (big code and packaging changes days before release) so I would prefer to do the right way in 2.9.0, if possible. One compromise for v2.8.0 would be to look into building a wheel and doing that on the build time, but still use current runner loading code in StackStorm/st2. This is not ideal, because once we install runners as Python packages in st2 virtual environment, most of the loading code in st2 is redundant (and could potentially cause issues), but this way we reduce the scope of work for v2.8.0. @bigmstone I saw the discussion on Slack, I'm fine with doing it in the Makefile which you linked where we already handle other wheels. One option would also be to not support st2kv functions in Orchestra in v2.8.0, but that's for others to decide. |
Member
|
Just to be clear, the change will still work for airgapped envs? |
enykeev
changed the title
Member
Author
|
@enykeev In theory it should since all other Orchestra requirements should already be installed in st2 virtualenv then (we are only running setup.py install to install stevedore plugins), but I'm with @armab on that one. Still messy and I would prefer to go with some other approach as described above. |
Member
Author
|
Superseded by #572. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #569 - it fixes path to the st2 virtual environment.