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

Added an idem exec and state module #57969

Closed
wants to merge 2 commits into from
Closed

Conversation

Akm0d
Copy link
Contributor

@Akm0d Akm0d commented Jul 17, 2020

What does this PR do?

fixes #57980
depends #57993

Adds an exec module and a state module for idem.
Idem execution modules and states can now be run through salt in a non-obtrusive way

Merge requirements satisfied?

Commits signed with GPG?

Yes

@Akm0d Akm0d requested a review from a team as a code owner July 17, 2020 01:33
@ghost ghost requested review from waynew and removed request for a team July 17, 2020 01:33
@Akm0d Akm0d self-assigned this Jul 17, 2020
@Akm0d Akm0d added this to the Approved milestone Jul 17, 2020
salt/states/idem.py Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
tests/integration/modules/test_idem.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
tests/integration/modules/test_idem.py Outdated Show resolved Hide resolved
tests/integration/modules/test_idem.py Outdated Show resolved Hide resolved
@Akm0d Akm0d requested review from waynew and s0undt3ch July 17, 2020 20:47
salt/states/idem.py Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
@Akm0d Akm0d requested a review from s0undt3ch July 17, 2020 21:07
@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label Jul 17, 2020
salt/states/idem.py Outdated Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
@max-arnold
Copy link
Contributor

If I understand this correctly, idem is asynchronous, right? How this would work with the planned Tornado upgrade where it is only possible to have a single event loop per thread?

@Akm0d
Copy link
Contributor Author

Akm0d commented Jul 18, 2020

If I understand this correctly, idem is asynchronous, right? How this would work with the planned Tornado upgrade where it is only possible to have a single event loop per thread?

This is executed in a separate process that is isolated from the original tornado loop

@max-arnold
Copy link
Contributor

This is executed in a separate process that is isolated from the original tornado loop

Does idem start this process somewhere, or it is a minion worker process? (I'm mostly asking this to educate myself, feel free to ignore)

@max-arnold
Copy link
Contributor

max-arnold commented Jul 18, 2020

Can Salt run states and/or execution modules in the main thread that has an existing event loop?

@thatch45
Copy link
Contributor

Since this is an execution module it will run inside of a worker process. Idem will run using the available loop that should be present when the worker is spun up.

@Akm0d Akm0d requested a review from s0undt3ch July 20, 2020 17:41
salt/states/idem.py Outdated Show resolved Hide resolved
salt/utils/idem.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

Just a question about the context manager usage.

tests/integration/states/test_idem.py Show resolved Hide resolved
salt/states/idem.py Outdated Show resolved Hide resolved
thatch45
thatch45 previously approved these changes Jul 20, 2020
@waynew
Copy link
Contributor

waynew commented Jul 31, 2020

I'd really like to do a deeper dive here (and maybe get some of the other more experienced team to review as well). Maybe we could review during your Twitch stream on Monday? That way you could walk us through the changes, your thoughts, any questions that you had as you were putting this together, and maybe we'd come up with questions as well. Or it will all make perfect sense and everyone will be on board 😂

Copy link
Collaborator

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

This now has conflicts

@Akm0d Akm0d closed this Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Execution-Module Magnesium Mg release after Na prior to Al State-Module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] Run idem states and modules from within salt
8 participants