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

[develop] Initial work to allow running multiple instances of a Salt engine #50059

Merged

Conversation

garethgreenaway
Copy link
Member

@garethgreenaway garethgreenaway commented Oct 15, 2018

What does this PR do?

Initial work to allow running multiple instances of a Salt engine

What issues does this PR fix or reference?

N/A

Tests written?

Yes.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Copy link
Contributor

@cachedout cachedout left a comment

LGTM so far.

Copy link
Contributor

@gtmanfred gtmanfred left a comment

nice

@cachedout
Copy link
Contributor

@cachedout cachedout commented Oct 18, 2018

Just to clarify, this is for running multiple instances of the same engine, yes?

@garethgreenaway
Copy link
Member Author

@garethgreenaway garethgreenaway commented Oct 18, 2018

@cachedout exactly.

@@ -46,10 +46,22 @@ def start_engines(opts, proc_mgr, proxy=None):
engine, engine_opts = next(iter(engine.items()))
else:
engine_opts = None
fun = '{0}.start'.format(engine)
engine_name = None
if engine_opts is not None and 'type' in engine_opts:
Copy link
Contributor

@rares-pop rares-pop Oct 19, 2018

Choose a reason for hiding this comment

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

where is that 'type' coming from? It's a new configuration for an engine?

Copy link
Member Author

@garethgreenaway garethgreenaway Oct 23, 2018

Choose a reason for hiding this comment

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

type would be a new configuration option in the engine configuration, so it would contain strings like slack or script depending on what type of engine the particular stanza in the configuration would be providing.

@garethgreenaway garethgreenaway force-pushed the allow_multiple_engine_instances branch 2 times, most recently from f56f5e2 to ec75a3c Compare Oct 28, 2018
@garethgreenaway garethgreenaway changed the title [WIP] Initial work to allow running multiple instances of a Salt engine [develop] Initial work to allow running multiple instances of a Salt engine Oct 29, 2018
@garethgreenaway garethgreenaway force-pushed the allow_multiple_engine_instances branch from 0c3bd91 to d84993b Compare Oct 29, 2018
@rallytime
Copy link
Contributor

@rallytime rallytime commented Oct 30, 2018

@rares-pop How does this look to you now?

@rares-pop
Copy link
Contributor

@rares-pop rares-pop commented Oct 30, 2018

@rallytime - it looks good! It adds a very nice functionality. I like 'engine_module' better than the previously 'type'. Good job @garethgreenaway!

@rallytime rallytime merged commit 30537f2 into saltstack:develop Oct 30, 2018
10 of 11 checks passed
@waynew waynew added this to PR needs port to master in PRs to port to master Oct 24, 2019
@garethgreenaway garethgreenaway added the has master-port label Jan 11, 2020
dwoz added a commit that referenced this issue Jan 13, 2020
@garethgreenaway garethgreenaway moved this from PR needs port to master to PR merged in PRs to port to master Mar 24, 2020
@garethgreenaway garethgreenaway moved this from PR merged to PR has port to master in PRs to port to master Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awesome has master-port
Projects
PRs to port to master
  
PR has port to master
Development

Successfully merging this pull request may close these issues.

None yet

5 participants