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

Remove deprecated argument include_type_name from ES calls #50285

Merged
merged 11 commits into from
Nov 12, 2019

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Nov 12, 2019

Summary

include_type_name was deprecated since v7.0 and was recently removed in v8.0 this PR.

force_memory_term_dictionary was deprecated as well.

ATM tests are breaking on master due to a snapshot update. This PR removes all usages of include_type_name and force_memory_term_dictionary.

mappings files which need to be updated:

  • x-pack/test/functional/es_archives/auditbeat/default/mappings.json
  • x-pack/test/functional/es_archives/dashboard/feature_controls/security/mappings.json
  • x-pack/test/functional/es_archives/dashboard/feature_controls/spaces/mappings.json
  • x-pack/test/functional/es_archives/discover/feature_controls/security/mappings.json
  • x-pack/test/functional/es_archives/discover/feature_controls/spaces/mappings.json
  • x-pack/test/functional/es_archives/infra/6.6.0/docker/mappings.json
  • x-pack/test/functional/es_archives/infra/7.0.0/hosts/mappings.json
  • x-pack/test/functional/es_archives/infra/legacy/mappings.json (unused?)
  • x-pack/test/functional/es_archives/monitoring/apm/mappings.json
  • x-pack/test/functional/es_archives/monitoring/basic_6.3.x/mappings.json
  • x-pack/test/functional/es_archives/monitoring/beats-with-restarted-instance/mappings.json
  • x-pack/test/functional/es_archives/monitoring/beats/mappings.json
  • x-pack/test/functional/es_archives/monitoring/ccr/mappings.json
  • x-pack/test/functional/es_archives/monitoring/logstash-pipelines/mappings.json
  • x-pack/test/functional/es_archives/monitoring/multi-basic/mappings.json
  • x-pack/test/functional/es_archives/monitoring/multicluster/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-basic-beats/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-green-gold/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-green-platinum/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-green-trial-two-nodes-one-cgrouped/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-red-platinum/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-three-nodes-shard-relocation/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-yellow-basic/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-yellow-platinum--with-10-alerts/mappings.json
  • x-pack/test/functional/es_archives/monitoring/singlecluster-yellow-platinum/mappings.json
  • x-pack/test/functional/es_archives/monitoring/standalone_cluster/mappings.json
  • x-pack/test/functional/es_archives/spaces/copy_saved_objects/mappings.json
  • x-pack/test/functional/es_archives/timelion/feature_controls/mappings.json
  • x-pack/test/functional/es_archives/uptime/pings/mappings.json
  • x-pack/test/functional/es_archives/visualize/default/mappings.json
  • x-pack/test/reporting/es_archives/bwc/6_2/mappings.json
  • x-pack/test/reporting/es_archives/bwc/6_3/mappings.json
  • x-pack/test/saved_object_api_integration/common/fixtures/es_archiver/saved_objects/spaces/mappings.json
  • x-pack/test/functional/es_archives/saved_objects_management/feature_controls/security/mappings.json

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@lizozom lizozom self-assigned this Nov 12, 2019
@lizozom lizozom added bug Fixes for quality problems that affect the customer experience v7.6.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:AppArch labels Nov 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

Copy link

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Changes LGTM, did not test locally

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Tested locally, Console changes LGTM.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rudolf
Copy link
Contributor

rudolf commented Nov 12, 2019

@elastic/stack-monitoring After changing the mappings file x-pack/test/functional/es_archives/monitoring/apm/mappings.json to remove the "doc" type so that it's compatible with v8.0.0 the test

it('should get apm instance data', async () => {
started failing. Can you please take a look?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rudolf
Copy link
Contributor

rudolf commented Nov 12, 2019

@lizozom

This failing test:

apis
       management
         index management
           settings
             should fetch an index settings:
     Error: Expected setting "force_memory_term_dictionary" not found

seems to be caused by the removal of that setting:
https://github.com/elastic/elasticsearch/blob/3ce7a37f1ff2ff4c24c97c60ce619f840ed1b7fb/docs/reference/migration/migrate_8_0/indices.asciidoc

So it should be safe to remove.

/CC @elastic/es-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom lizozom merged commit 19f7e99 into elastic:master Nov 12, 2019
Copy link
Contributor

@rudolf rudolf left a comment

Choose a reason for hiding this comment

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

Even when ignoring whitespace there's so much formatting changes it's hard to review every line, but with ignoring the mappings changes everything looks good, so with CI being green I think this is good to merge 👍

@lizozom lizozom removed the v7.6.0 label Nov 12, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 13, 2019
* upstream/master:
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 13, 2019
…-fallback

* 'master' of github.com:elastic/kibana:
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 13, 2019
…ger-ace-theme

* 'master' of github.com:elastic/kibana: (56 commits)
  [ML] Server info service refactor (elastic#50302)
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
  [ML] Make navigation in tests more stable (elastic#50132)
  Migrate authorization subsystem to the new platform.  (elastic#46145)
  Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063)
  Update dependency @elastic/charts to v14 (elastic#49947)
  ...
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 14, 2019
…0285)

This PR is merged with an error, because these errors happen on master as well. @flash1293 is working on fixing those.

This one should allow us running with the latest es snapshot 



* deprecate include_type_name

* include_type_name

* remove doc from mappings

* Updated timelion mapping

* Updated spaces and uptime mapping

* monitoring apm mapping

* Updated more mappings

* 2 more mappings

* Updated reporting mappings after syncing with @gammon

* Removed deprecated setting
@lizozom lizozom deleted the deprecate/remove-include_type_name branch November 14, 2019 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants