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

Cleanup action task params objects after successful execution #55227

Merged
merged 15 commits into from
Jan 27, 2020

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Jan 17, 2020

Resolves #49408.

In this PR, I'm making the task runner cleanup action_task_params saved objects after successful execution. Successful execution is determined when status: 'ok' is returned from an execution.

To support integration tests, I've added a createdAt field within the action_task_params mappings. This allows tests to filter out records previously created which are still there from failed executions. I also changed a fixture action type to return a status (status: ok) as it was missing.

@mikecote mikecote added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Jan 17, 2020
@mikecote mikecote self-assigned this Jan 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, but added a note about reusing an existing SO field instead of adding createdAt, and am wondering about cleanup of action errors w/retries

x-pack/legacy/plugins/actions/mappings.json Outdated Show resolved Hide resolved
@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

@mikecote mikecote requested a review from a team January 27, 2020 13:11
@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mikecote mikecote merged commit 8fe39ae into elastic:master Jan 27, 2020
mikecote added a commit to mikecote/kibana that referenced this pull request Jan 27, 2020
…c#55227)

* Cleanup action task params saved objects after use

* Fix jest tests

* Add integration test to ensure object gets cleaned up

* Add unit tests

* Fix comment

* Re-use updated_at instead of creating createdAt

* Consider null/undefined returned from executor as success as well

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mikecote added a commit that referenced this pull request Jan 27, 2020
#56058)

* Cleanup action task params saved objects after use

* Fix jest tests

* Add integration test to ensure object gets cleaned up

* Add unit tests

* Fix comment

* Re-use updated_at instead of creating createdAt

* Consider null/undefined returned from executor as success as well

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 28, 2020
…ve-out-legacy

* 'master' of github.com:elastic/kibana: (187 commits)
  [ML] Reseting categorization validation if category field is cleared (elastic#56029)
  [SIEM] Fields browser readable (elastic#56000)
  [docs] Remove unused callout (elastic#56032)
  Refactor saved object management registry usage (elastic#54155)
  [SIEM][Detection Engine] critical blocker, updates the pre-packaged rules, removes dead ones, adds license file (elastic#56090)
  Fix failing snapshot artifact tests when using env var (elastic#56063)
  Fix Github PR comment formatting (elastic#56078)
  [Maps] fix join metric field selection bugs (elastic#56044)
  Create a new menu for observability links (elastic#54847)
  [SIEM] [Detection Engine] Fixes histogram intervals  (elastic#55969)
  make test less flaky by retrying if list is re-rendered (elastic#55949)
  Remove matrix build support (elastic#54202)
  Add animation to service map layout (elastic#56042)
  [Canvas] Remove Angular and unnecessary reporting config from Canvas (elastic#54050)
  [Uptime] Simplify snapshot max to Infinity (elastic#55931)
  [Uptime] Reintroduce a column for url (elastic#55451)
  Cleanup action task params objects after successful execution (elastic#55227)
  [CI] Retry flaky tests (elastic#53961)
  Expose NP FieldFormats service to server side (elastic#55419)
  [Endpoint] EMT-65: make endpoint data types common, restructure (elastic#54772)
  ...

# Conflicts:
#	src/legacy/core_plugins/console/public/np_ready/application/components/split_panel/__snapshots__/split_panel.test.tsx.snap
#	src/legacy/core_plugins/console/public/np_ready/application/components/split_panel/containers/panel.tsx
#	src/legacy/core_plugins/console/public/np_ready/application/components/split_panel/context.tsx
#	src/legacy/core_plugins/console/public/np_ready/application/components/split_panel/index.ts
#	src/legacy/core_plugins/console/public/np_ready/application/components/split_panel/split_panel.test.tsx
#	src/legacy/ui/public/vis/editors/default/default_editor.tsx
#	src/plugins/console/public/application/components/split_panel/__snapshots__/split_panel.test.tsx.snap
#	src/plugins/console/public/application/components/split_panel/components/resizer.tsx
#	src/plugins/console/public/application/components/split_panel/containers/panel.tsx
#	src/plugins/console/public/application/components/split_panel/containers/panel_container.tsx
#	src/plugins/console/public/application/components/split_panel/context.tsx
#	src/plugins/console/public/application/components/split_panel/index.ts
#	src/plugins/console/public/application/components/split_panel/registry.ts
#	src/plugins/console/public/application/components/split_panel/split_panel.test.tsx
#	src/plugins/kibana_react/public/split_panel/__snapshots__/split_panel.test.tsx.snap
#	src/plugins/kibana_react/public/split_panel/containers/panel.tsx
#	src/plugins/kibana_react/public/split_panel/context.tsx
#	src/plugins/kibana_react/public/split_panel/index.ts
#	src/plugins/kibana_react/public/split_panel/split_panel.test.tsx
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 28, 2020
* master: (77 commits)
  [ML] Reseting categorization validation if category field is cleared (elastic#56029)
  [SIEM] Fields browser readable (elastic#56000)
  [docs] Remove unused callout (elastic#56032)
  Refactor saved object management registry usage (elastic#54155)
  [SIEM][Detection Engine] critical blocker, updates the pre-packaged rules, removes dead ones, adds license file (elastic#56090)
  Fix failing snapshot artifact tests when using env var (elastic#56063)
  Fix Github PR comment formatting (elastic#56078)
  [Maps] fix join metric field selection bugs (elastic#56044)
  Create a new menu for observability links (elastic#54847)
  [SIEM] [Detection Engine] Fixes histogram intervals  (elastic#55969)
  make test less flaky by retrying if list is re-rendered (elastic#55949)
  Remove matrix build support (elastic#54202)
  Add animation to service map layout (elastic#56042)
  [Canvas] Remove Angular and unnecessary reporting config from Canvas (elastic#54050)
  [Uptime] Simplify snapshot max to Infinity (elastic#55931)
  [Uptime] Reintroduce a column for url (elastic#55451)
  Cleanup action task params objects after successful execution (elastic#55227)
  [CI] Retry flaky tests (elastic#53961)
  Expose NP FieldFormats service to server side (elastic#55419)
  [Endpoint] EMT-65: make endpoint data types common, restructure (elastic#54772)
  ...
@mikecote mikecote added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:fix review Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup action_task_params object after task successfully executed
5 participants