Skip to content
This repository was archived by the owner on Jan 29, 2022. It is now read-only.

Async routes#49

Merged
dhollinger merged 2 commits intovoxpupuli:masterfrom
dhollinger:async_routes
Apr 24, 2018
Merged

Async routes#49
dhollinger merged 2 commits intovoxpupuli:masterfrom
dhollinger:async_routes

Conversation

@dhollinger
Copy link
Copy Markdown
Member

@dhollinger dhollinger commented Apr 24, 2018

Removed open3 calls in the run_command method as it was preventing the application from forking system calls. The logging of commands still exists within the application.

Moved the process fork from the run_command method into the method call in each POST route. This allows for not only running system calls asynchronously, but also allows for the API calls to run asynchronously.

This solves an issue where Gitlab and Github webhooks expect a response within 10 seconds while an r10k or mco run could take longer to return.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 24, 2018

Coverage Status

Coverage increased (+0.8%) to 51.952% when pulling 108e404 on dhollinger:async_routes into d62426a on voxpupuli:master.

module_name = sanitize_input(module_name)
LOGGER.info("Deploying module #{module_name}")
deploy_module(module_name)
Prcocess.detach(fork { deploy_module(module_name) })
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Prco... typo

message = "forked: #{command}"
exec "#{command} &"
exit_status = 0
raise "#{stdout}\n#{stderr}" if exit_status != 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this line redundant if you explicitly set exit_status = 0?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ekohl Would it be better to remove that or replace exit_status = 0 with exit_status = exec "echo $?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd remove it because it's forked and the output is useless:

ekohl@wisse ~ $ false &
[1] 13302
[1]+  Exit 1                  false
ekohl@wisse ~ $ echo $?
0
ekohl@wisse ~ $ /usr/bin/sfdsdfsfs &
[1] 13324
bash: /usr/bin/sfdsdfsfs: No such file or directory
[1]+  Exit 127                /usr/bin/sfdsdfsfs
ekohl@wisse ~ $ echo $?
0

@dhollinger dhollinger merged commit b7f1125 into voxpupuli:master Apr 24, 2018
@dhollinger dhollinger deleted the async_routes branch April 24, 2018 18:57
@alexjfisher
Copy link
Copy Markdown
Member

Is this why two instances of r10k can now run simultaneously? (which is bad).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants