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

Changes x-pack action executor to return result. #39820

Merged
merged 4 commits into from
Jul 1, 2019

Conversation

pmuellr
Copy link
Member

@pmuellr pmuellr commented Jun 27, 2019

Previously, the result was never captured. It's now captured, and
returned if it's JSONable. If not JSONable a {succcess: true}
object is returned with a 200 response (used to be 204).

Also added logging for action execution calls, in lieu of having an
action history available.

Drive-by:

  • services.log was set to the unbound Hapi server.log method, so
    places where it was set were changed to bind it to the server.

Previously, the result was never captured.  It's now captured, and
returned if it's JSONable.  If not JSONable a `{succcess: true}`
object is returned with a 200 response (used to be 204).

Also added logging for action execution calls, in lieu of having an
action history available.

Drive-by:

- `services.log` was set to the unbound Hapi server.log method, so
  places where it was set were changed to bind it to the server.
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services

@pmuellr
Copy link
Member Author

pmuellr commented Jun 28, 2019

jenkins retest this please

@elasticmachine
Copy link
Contributor

💔 Build Failed

@pmuellr
Copy link
Member Author

pmuellr commented Jun 28, 2019

jenkins retest this please

@pmuellr pmuellr marked this pull request as ready for review June 28, 2019 13:27
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pmuellr pmuellr requested a review from mikecote June 28, 2019 14:57
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

LGTM, just one comment about logging.


if (error != null) {
services.log(
['info', 'x-pack', 'actions'],
Copy link
Contributor

Choose a reason for hiding this comment

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

info seems like the wrong log level for an error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes. And for the info one below (success), wondering if that should be a debug. How chatty is the Kibana log? Seems usually quiet to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah debug is probably better for the one below.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pmuellr pmuellr removed the request for review from mikecote July 1, 2019 16:47
@pmuellr pmuellr merged commit c4749b7 into elastic:master Jul 1, 2019
@pmuellr pmuellr deleted the action/log-action-execution branch July 1, 2019 17:00
pmuellr added a commit to pmuellr/kibana that referenced this pull request Jul 1, 2019
* Changes x-pack action executor to return result.

Previously, the result was never captured.  It's now captured, and
returned if it's JSONable.  If not JSONable a `{succcess: true}`
object is returned with a 200 response (used to be 204).

Also added logging for action execution calls, in lieu of having an
action history available.

Drive-by:

- `services.log` was set to the unbound Hapi server.log method, so
  places where it was set were changed to bind it to the server.
pmuellr added a commit that referenced this pull request Jul 1, 2019
* Changes x-pack action executor to return result.

Previously, the result was never captured.  It's now captured, and
returned if it's JSONable.  If not JSONable a `{succcess: true}`
object is returned with a 200 response (used to be 204).

Also added logging for action execution calls, in lieu of having an
action history available.

Drive-by:

- `services.log` was set to the unbound Hapi server.log method, so
  places where it was set were changed to bind it to the server.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants