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 log_and_exit utility method #414

Merged
merged 1 commit into from
Aug 29, 2018
Merged

Conversation

mkanoor
Copy link
Contributor

@mkanoor mkanoor commented Aug 28, 2018

This utility method allows a user to log and exit with a single method call.
It also tests the exit call from a spec

@mkanoor
Copy link
Contributor Author

mkanoor commented Aug 28, 2018

@bdunne
Please review this is a tricky exit call in an Automate Method, which we have refrained from. I remember you doing some research on this in the past.

#
def self.log_and_exit(msg, exit_code, handle = $evm)
handle.log('info', "Script ending #{msg} code : #{exit_code}")
exit(exit_code)
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gmcculloug we recently fixed a bug in how MIQ_STOP can be used to a state machine which requires us to use the exit system call. Till now we have refrained from using exits in the newer methods because that causes the spec to exit. This is more a way to ensure we can test methods with exit in them.

@gmcculloug gmcculloug requested a review from bdunne August 28, 2018 21:09
@gmcculloug gmcculloug self-assigned this Aug 28, 2018
@bdunne
Copy link
Member

bdunne commented Aug 28, 2018

@mkanoor It looks like you have a "Fix rubocop" commit, can you squash that?

@miq-bot
Copy link
Member

miq-bot commented Aug 28, 2018

Checked commit mkanoor@a3a7d61 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

spec/content/automate/ManageIQ/System/CommonMethods/Utils.class/methods/log_object_spec.rb

@bdunne
Copy link
Member

bdunne commented Aug 28, 2018

I forget why we need to call exit rather than sending the "exit code" across the DRB connection and letting the script end

@mkanoor
Copy link
Contributor Author

mkanoor commented Aug 28, 2018

@bdunne These are external customer written scripts that call the exit at many different points in their code, the exit(rc) naturally flows back thru the exit status
https://github.com/ManageIQ/manageiq-automation_engine/blob/608d372004d7b50b116ab0fb375a0934cc2f1f69/lib/miq_automation_engine/engine/miq_ae_engine/miq_ae_method.rb#L199

exit is a normal way for customers to end their scripts
If we have to teach them to do $evm.exit_code(8) that would be cumbersome.

@bdunne
Copy link
Member

bdunne commented Aug 29, 2018

Okay, well the post script will re-raise SystemExit, so I don't see any problems

@gmcculloug gmcculloug merged commit b0a951e into ManageIQ:master Aug 29, 2018
@gmcculloug gmcculloug added this to the Sprint 94 Ending Sept 10, 2018 milestone Aug 29, 2018
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.

4 participants