Skip to content

Conversation

@kbrock
Copy link
Member

@kbrock kbrock commented Apr 30, 2024

depends upon:

Overview

  • Current runners link to an external process that manages the runners.
  • Current runners are asynchronous

So that begs to ask the question on how to implement a runner that manages its own subprocesses.

Implementation

I came up with a synchronous and an asynchronous implementation. They get converted across with a commit.

I feel the synchronous implementation may be the best one, otherwise it has a bunch of threads hanging around.

commits:

  • dependency
  • Synchronous implementation
  • asynchronous implementation

Issues

This doesn't work perfectly since AwesomeSpawn escapes $VAR and that makes this pretty useless.

But the goal was more focused on the Threads and stuff rather than the actual awesome spawn part.

@kbrock kbrock requested review from Fryguy and agrare as code owners April 30, 2024 18:41
@kbrock kbrock force-pushed the awesome_runner branch 2 times, most recently from 66d040b to f6c4259 Compare April 30, 2024 20:18
@kbrock
Copy link
Member Author

kbrock commented Apr 30, 2024

updates:

  • fixed wait code to run for multiple schemes
  • many cops

@kbrock
Copy link
Member Author

kbrock commented Apr 30, 2024

update:

  • fix comments around multi threading

@kbrock kbrock force-pushed the awesome_runner branch from a2a833c to b8d9e89 Compare May 1, 2024 02:32
@Fryguy Fryguy changed the title [POC] Awesome runner [WIP] [POC] Awesome runner May 1, 2024
@miq-bot miq-bot added the wip label May 1, 2024
require "concurrent/array"

module Floe
class AwesomeProcess < Thread
Copy link
Member Author

Choose a reason for hiding this comment

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

would have preferred not using a thread per process and not using an AwesomeSpan centric model.

@kbrock kbrock force-pushed the awesome_runner branch 2 times, most recently from 436600d to af7918d Compare May 2, 2024 18:15
@kbrock kbrock force-pushed the awesome_runner branch 2 times, most recently from f518699 to 61d4123 Compare June 7, 2024 19:08
@kbrock
Copy link
Member Author

kbrock commented Jun 7, 2024

update:

  • rebase

update:

  • fixed spec (due to rebase)

@miq-bot
Copy link
Member

miq-bot commented Jun 20, 2024

Checked commits kbrock/floe@e25d67e~...514bed0 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
5 files checked, 0 offenses detected
Everything looks fine. 🍰

@miq-bot
Copy link
Member

miq-bot commented Sep 23, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2024

This pull request is not mergeable. Please rebase and repush.

@miq-bot
Copy link
Member

miq-bot commented Feb 3, 2025

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@miq-bot
Copy link
Member

miq-bot commented May 5, 2025

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@kbrock
Copy link
Member Author

kbrock commented Sep 10, 2025

update:

  • rebased

still WIP

@Fryguy
Copy link
Member

Fryguy commented Sep 10, 2025

naming-wise I don't like the awesome part - externally awesome:// means nothing. If this is basically just shell execution, let's call it shell:// or if it's bash specific then bash://

Comment on lines +16 to +21
"Resource": "awesome://ls -l Gemfile",
"Comment": "awesome://ls -l $FILENAME",
"Next": "c",
"Parameters": {
"FILENAME" : "Gemfile"
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking we should adhere to the AwesomeSpawn style of params handling, so something roughtly like:

Suggested change
"Resource": "awesome://ls -l Gemfile",
"Comment": "awesome://ls -l $FILENAME",
"Next": "c",
"Parameters": {
"FILENAME" : "Gemfile"
}
"Resource": "awesome://ls",
"Comment": "awesome://ls -l $FILENAME",
"Next": "c",
"Parameters": {
"-l": null,
"Gemfile": null
}

Though, I'm not sure how we would handle duplicate parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

aah. I think $FILENAME did not work and that is why I put that comment in there.
Couldn't get shell env var expansion working. (There was a legit use case in there)

@Fryguy
Copy link
Member

Fryguy commented Sep 12, 2025

FWIW, I would not want this in ManageIQ, but it could be useful outside of ManageIQ. The reason is because we run user-created workflows, and this would give access to the underlying OS/filesystem, which would open up potential security issues. However, if I wanted to automate, say, my own personal stuff on my computer, this would be very useful.

@kbrock
Copy link
Member Author

kbrock commented Sep 12, 2025

I was thinking for tests. But yea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants