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

80697: ignore 404, check if there are transforms in results. #80721

Conversation

nnamdifrankie
Copy link
Contributor

@nnamdifrankie nnamdifrankie commented Oct 15, 2020

#80697

Summary

  • ignore missing transform even when present in Fleet.
  • check that there is transform in response before deleting index attached to transform.

Checklist

Delete any items that are not applicable to this PR.

@nnamdifrankie nnamdifrankie added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Oct 15, 2020
@nnamdifrankie nnamdifrankie marked this pull request as ready for review October 16, 2020 01:33
@nnamdifrankie nnamdifrankie requested a review from a team October 16, 2020 01:33
@nnamdifrankie
Copy link
Contributor Author

@elastic/endpoint-management

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Oct 16, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

Code LGTM. Left a notes with suggested fixes re: logger/mocking. Please apply before merging.


export const getAsset = (path: string): Buffer => {
return Registry.getAsset(path);
};

export const getLogger = (): Logger => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this and instead import appContextService where it's needed to remain consistent with the rest of the plugin.

@@ -11,6 +11,7 @@ jest.mock('../../packages/get', () => {
jest.mock('./common', () => {
return {
getAsset: jest.fn(),
getLogger: jest.fn(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please drop this and add something like

appContextService.start(createAppContextStartContractMock());
in a before/beforeEach

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@nnamdifrankie nnamdifrankie merged commit cba7e58 into elastic:master Oct 16, 2020
@nnamdifrankie nnamdifrankie deleted the 80697-ignore-missing-transform-check-before-deleting branch October 16, 2020 15:28
nnamdifrankie added a commit to nnamdifrankie/kibana that referenced this pull request Oct 16, 2020
…tic#80721)

[Ingest]: ignore 404, check if there are transforms in results
nnamdifrankie added a commit that referenced this pull request Oct 16, 2020
…) (#80853)

[Ingest]: ignore 404, check if there are transforms in results
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 19, 2020
* master: (51 commits)
  [Discover] Unskip flaky test (elastic#80670)
  Fix security solution template label (elastic#80754)
  [Ingest]: ignore 404, check if there are transforms in results. (elastic#80721)
  Moving loader to logo in header, add a slight 250ms pause (elastic#78879)
  [Security Solution][Cases] Fix bug with case connectors (elastic#80642)
  Update known-plugins.asciidoc (elastic#75388)
  [Lens] Add median operation (elastic#79453)
  Fix navigateToApp logic when navigating to the current app. (elastic#80809)
  [Visualizations] Fix bad color mapping with multiple split series (elastic#80801)
  [ILM] Add esErrorHandler for the new es js client (elastic#80302)
  Fix codeowners (elastic#80826)
  skip flaky suite (elastic#79463)
  [Timelion] Remove kui usage (elastic#80287)
  [Ingest Manager] add skipIfNoDockerRegistry to package_install_complete test (elastic#80779)
  [Alerting UI] Disable "Save" button for Alerts with broken Connectors (elastic#80579)
  Allow the default space to be accessed via `/s/default` (elastic#77109)
  Add script to identify plugin dependencies for TS project references migration (elastic#80463)
  [Search] Client side session service (elastic#76889)
  feat: 🎸 add separator for different context menu groups (elastic#80498)
  Lazy load reporting (elastic#80492)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants