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

[enterprise search]: disallow removing last index from engine #150464

Conversation

TattdCodeMonkey
Copy link
Contributor

Summary

Removing the option to remove the last index from an engine. We don't allow creating engines with no indices, so allowing a user to remove all indices from an engine doesn't make sense either.

Screenshots

Engine with 1 index
image
Engines with >1 index (has remove index action)
image

@TattdCodeMonkey TattdCodeMonkey added release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.7.0 labels Feb 7, 2023
@TattdCodeMonkey TattdCodeMonkey requested a review from a team February 7, 2023 17:57
Copy link
Member

@kderusso kderusso left a comment

Choose a reason for hiding this comment

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

Interestingly enough, I don't think we technically enforce this constraint on PUT requests to the backend API.

I think this is OK to do for now, but we should think if there are any use cases to having an Engine lying around with no more indices. Perhaps the engine is going away but they're still hanging onto analytics? Would that be a valid use case? If there truly are no valid use cases for this, we should consider on the backend either a) disallowing removal of the last index from the engine or b) acting the way aliases do and remove the engine itself if the last index is removed (I don't think that we want to do b in practice).

Copy link

@sloanelybutsurely sloanelybutsurely left a comment

Choose a reason for hiding this comment

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

is there a ticket to prevent this at the API level also?

Removing the option to remove the last index from an engine. We don't
allow creating engines with no indices, so allowing a user to remove all
indices from an engine doesn't make sense either.
@TattdCodeMonkey TattdCodeMonkey force-pushed the rnorris/feat/engines-disallow-removing-final-index branch from d91e638 to 1d11305 Compare February 7, 2023 18:31
@TattdCodeMonkey
Copy link
Contributor Author

@sloanelybutsurely there is not a ticket for preventing this at the API-level. It has come up in a few meetings and wanted to go ahead and get this in before feature freeze. But we should definitely talk more about it.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.1MB 2.1MB +23.0B

History

  • 💔 Build #106210 failed d91e638f22906ffc2d3beb5c97e3d4d00893ef2f

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

@TattdCodeMonkey TattdCodeMonkey merged commit d223f42 into elastic:main Feb 7, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 7, 2023
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 8, 2023
* main: (187 commits)
  [APM] Removing the icon for tech preview and fixing some of the alert badges (elastic#150528)
  [Cloud Posture][Bug]added fix for resource tab + small css fix on CSPM onboarding (elastic#149997)
  Making maxTransactionGroupsExceeded false by default (elastic#150458)
  [Fleet] refactor install registry and upload to extract common logic (elastic#150444)
  [ftr tests] split x-pack functional_with_es_ssl config (elastic#150416)
  [APM] switch get environment function to use terms_enum api (elastic#150175)
  [Unified search] Fixes ally issues (elastic#150411)
  [Synthetics] Fix overview status query (elastic#150285)
  [api-docs] 2023-02-08 Daily api_docs build (elastic#150518)
  [canvas] Run tests against @kbn/handlebars compileAST function (elastic#150439)
  [RAM] Bring flapping status and settings in o11y (elastic#150483)
  [data view mgmt] fix field refresh when index pattern is changed. (elastic#150403)
  [RAM] Allow alert table to show new alert status on apm (elastic#150500)
  [Synthetics] errors - adjust empty state content (elastic#150455)
  Uncomment tests (elastic#150481)
  [TIP] fix broken cypress tests after change made in cases plugin (elastic#150479)
  [enterprise search]: disallow removing last index from engine (elastic#150464)
  [Security Solution][Exceptions] Rule exceptions TTL - Expiration (elastic#145180)
  Adds link to Jan 2023 contributors newsletter. (elastic#150259)
  [Security Solution][Alerts] Fix bug when suppression has both created and updated alerts (elastic#150236)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants