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 support for multiple action runners (runner modules) inside a single runner Python package #3999

Merged
merged 33 commits into from
Feb 16, 2018

Conversation

Kami
Copy link
Member

@Kami Kami commented Feb 14, 2018

This pull request adds support for multiple runners and runner modules inside a single action runner Python package.

We already partially introduced support for that in the past during runner refactor done by @bigmstone by allowing "runner.yaml" metadata file to be an array and contain multiple entries (one per runner).

The problem is that support was not fully implemented so we needed to resort to handling multiple runners inside a single runner module inside the code using if checks. That's not ideal and ability to utilize inheritance is better and results in a cleaner code.

I already partially introduced support for that when working on serverless stuff and making runners re-distributable and standalone Python packages. This pull request finishes what I started there.

I originally wanted to postpone this, but I need this code change to be able to continue work on #3997 (one of the changes I'm working in there only applies to local shell script, but not local shell command runner and inheritance makes code a lot more cleaner).

In addition to setting up ground work for multiple runner modules per Python package it also moves local and windows runners to that model.

Another benefit this change brings is that now we can put common code shared by local or remote runners in that runner package. Previously it needed to live inside st2common which was far from ideal.

TODO

  • Move local runners to the new model
  • Move windows runners to the new model
  • Move remote runners to the new model
  • Remove now unneeded backward compatibility code from runner package __init__.py files

package.

This means we now support multiple runner modules inside a single Python
package which provides for a better abstraction then the one we
currently use (multiple runners inside a single Python module or package
per runner even for related runners).
@Kami Kami added this to the 2.7.0 milestone Feb 14, 2018
@@ -19,4 +19,4 @@
# This means you can either do "from runner_name import RunnerClass" (old way, don't do that)
# or "from runner_name.runner_name import RunnerClass"
from __future__ import absolute_import
from .local_runner import * # noqa
Copy link
Member Author

Choose a reason for hiding this comment

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

I will actually get rid of this backward compatibility stuff. It will make things nicer and we don't need it anymore, AFAIK.

@Kami Kami requested a review from bigmstone February 15, 2018 12:16
@Kami Kami changed the title [WIP] Add support for multiple action runners (runner modules) inside a single runner Python package Add support for multiple action runners (runner modules) inside a single runner Python package Feb 15, 2018
@Kami
Copy link
Member Author

Kami commented Feb 15, 2018

Just a heads up - once this PR is merged, you will need to re-run make virtualenv make target locally to re-create virtualenv/bin/activate file with updated PYTHONPATH and then run source virtualenv/bin/activate.

Copy link
Contributor

@bigmstone bigmstone left a comment

Choose a reason for hiding this comment

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

This is a good improvement. 👍

@Kami Kami merged commit b38fd38 into master Feb 16, 2018
@Kami Kami deleted the multiple_runner_modules_per_runner_package branch February 16, 2018 12:45
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.

None yet

2 participants