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

Hooks #190

Merged
merged 23 commits into from
Aug 30, 2016
Merged

Hooks #190

merged 23 commits into from
Aug 30, 2016

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented Aug 23, 2016

Hooks

Storyline: #62

This PR introduces hooks, processes to be executed by gh-ost at particular points of interest (e.g. startup, validated, starting postpone, success, status etc.)

  • contributed code is using same conventions as original code
  • code is formatted via gofmt (please avoid goimports)
  • code is built via ./build.sh
  • code is tested via ./test.sh

@shlomi-noach
Copy link
Contributor Author

shlomi-noach commented Aug 25, 2016

cmd := exec.Command(hook)
cmd.Env = this.applyEnvironmentVairables(extraVariables...)

if err := cmd.Run(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be helpful to pipe hook output (stdout & stderr) to gh-ost's standard output, especially for troubleshooting hook errors.

This might be out of scope of this PR. If so, and you are interested in the idea, I am happy to submit a PR once this is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. If this is to be done, it will be done in this PR.
Is there any sense in having this configurable?

Copy link
Contributor

Choose a reason for hiding this comment

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

I lean towards not having it configurable until someone needs it. An operator can simply choose not to output anything from the hook executable.

@shlomi-noach
Copy link
Contributor Author

hook stdout & stderr are now written onto gh-ost's stderr.

@shlomi-noach
Copy link
Contributor Author

Insofar this is looking good in testing. Merging, and will iterate if need be.

@shlomi-noach shlomi-noach merged commit 176aabe into master Aug 30, 2016
@shlomi-noach shlomi-noach deleted the hooks branch August 30, 2016 07:01
@shlomi-noach shlomi-noach changed the title WIP: Hooks Hooks Aug 30, 2016
@shlomi-noach
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants