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

Implement AwesomeSpawn.run_detached #32

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Aug 26, 2015

Add ability to detach a command but use all the command line generation goodness of AwesomeSpawn.run.

This also serves as a best practice to collect all the methods necessary to call spawn.

I noticed that we will probably need a run_in_sub_process / wait_for kind of method.

Was about to run some tests, but wanted to check in first on what I had so far

/cc @Fryguy


options[[:out, :err]] = ["/dev/null", "w"] unless (options.keys.flatten & [:out, :err]).any?
options[:pgroup] = true unless options.key?(:pggroup)
options[:pggroup_new] = true unless options.key?(:pggroup_new)
Copy link
Member

Choose a reason for hiding this comment

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

I think there are typos here and in all the docs s/pggroup/pgroup/

Choose a reason for hiding this comment

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

Recommend replacing "/dev/null" with IO::NULL.

Copy link
Member Author

Choose a reason for hiding this comment

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

looks like it is new_pgroup vs pgroup_new too. thnx

@Fryguy
Copy link
Member

Fryguy commented Aug 27, 2015

Been hoping for this method to be added! Glad it's coming together

@@ -101,6 +101,29 @@ def run!(command, options = {})
command_result
end

# Execute `command` in a detached manner
# by default the :err, and :out are redirected to `/dev/null`

Choose a reason for hiding this comment

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

"redirected to the null device".

def detach(env, command, options)
pid = Kernel.spawn(env, command, options)
Process.detach(pid)
pid

Choose a reason for hiding this comment

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

This can simplified to Process.detach(pid).pid

Copy link
Member Author

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

next rev

@coveralls
Copy link

coveralls commented Jul 8, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 14f214e on kbrock:detached into f5538f6 on ManageIQ:master.

@chessbyte
Copy link
Member

@kbrock how much work is it to bring this ancient PR over the finish line?

@kbrock kbrock changed the title [WIP] Implement AwesomeSpawn.detach Implement AwesomeSpawn.detach Oct 16, 2020
@miq-bot miq-bot removed the wip label Oct 16, 2020
this spawns a thread but runs in a separate
process.

It can run in the same or a different
@miq-bot
Copy link
Member

miq-bot commented Oct 16, 2020

Checked commit kbrock@14f214e with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@kbrock
Copy link
Member Author

kbrock commented Oct 16, 2020

Don't exactly remember why this was WIP.

I fixed to run on a mac. Documentation for Kernel.spawn was a little ambigious in terms of which parameters were allowed in certain environments. I have not tested the windows environment.

It spawns off the command line and I am un-WIPing it.

@Fryguy Fryguy merged commit 14b0e10 into ManageIQ:master Oct 19, 2020
@Fryguy Fryguy self-assigned this Oct 19, 2020
@kbrock kbrock deleted the detached branch July 14, 2022 18:31
@agrare
Copy link
Member

agrare commented Nov 1, 2023

Hey @bdunne any chance we can get a release that includes this? I haven't looked at what else landed in master but I'd like to use this instead of Process.detach(Kernel.spawn(AwesomeSpawn.build_command_line()))

@Fryguy
Copy link
Member

Fryguy commented Nov 1, 2023

I can take a look.

@Fryguy
Copy link
Member

Fryguy commented Nov 1, 2023

I just noticed there aren't specs for this. I can release anyway, but would feel more comfortable using it if we had some specs.

@Fryguy Fryguy changed the title Implement AwesomeSpawn.detach Implement AwesomeSpawn#run_detached Nov 1, 2023
@Fryguy Fryguy changed the title Implement AwesomeSpawn#run_detached Implement AwesomeSpawn.run_detached Nov 1, 2023
@Fryguy
Copy link
Member

Fryguy commented Nov 1, 2023

Released in 1.6.0, but of course I typod it in the commit message / changelog. Fixed the changelog in #71

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.

9 participants