Implemented XContent serialisation for GetIndexResponse by sohaibiftikhar · Pull Request #31675 · elastic/elasticsearch

sohaibiftikhar · 2018-06-29T01:24:34Z

This PR does the server side work for adding the Get Index API to the REST high-level-client.
A follow up would be the client side changes.

elasticmachine · 2018-06-29T02:27:13Z

Pinging @elastic/es-core-infra

tvernum · 2018-06-29T02:27:33Z

@elasticmachine test this please

javanna · 2018-06-29T13:09:18Z

add to whitelist

nik9000

I left a question inline about one part of the change. The rest looks right to me, I just don't know the particulars well enough to be sure about one thing so I left a question about it. The rest looks right to me.

Do you want to backport this to 6.x so you can backport #31703 to 6.4?

nik9000 · 2018-07-02T12:45:24Z

server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java

This is replacing this: indexScopedSettings.diff(settings, RestGetIndicesAction.this.settings) which doesn't look 100% the same. It feels like EMPTY should be the cluster settings.

To be honest I am not entirely sure about this either. I was following the example here.

Ok. I'll play with it a little myself and see if I can prove to myself it doesn't matter. Or that it does.

OK! That was pretty quick! I think this change is ok because of this line.

Thanks for the explanation!

nik9000 · 2018-07-02T13:52:26Z

server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java

I believe it should be safe to do Objects.hash(Arrays.hashCode(indices), aliases, mappings, settings, defaultSettings)

I believe so as well. I don't remember why I did it like this anymore. I will make the change.

sohaibiftikhar · 2018-07-02T14:15:42Z

@nik9000 Yes, this needs to be backported. I think we need to do it something like this?

nik9000 · 2018-07-02T15:01:08Z

@nik9000 Yes, this needs to be backported. I think we need to do it something like this?

++

This commit does the all the server side work adding the Get Index API to the REST high-level-client. A follow up would be the client side changes

-- will be reenabled after backport

sohaibiftikhar · 2018-07-03T10:45:28Z

After changing the version to 6.4. The mixed-cluster-tests failed as expected.
I turned off BWC checks now. Waiting for the build to go green.

I believe the following steps need to be taken now:

This needs to be merged to master as is (with BWC checks disabled).
Merged to 6.x by cherry-picking all commits except a300e1d which disabled the BWC check.
Re-enable BWC checks on master with a separate PR.

@nik9000 My question is what would happen if some other PR gets a green build with the BWC checks disabled before step 3 is completed. Will that be merged?

nik9000 · 2018-07-03T12:04:58Z

@nik9000 My question is what would happen if some other PR gets a green build with the BWC checks disabled before step 3 is completed. Will that be merged?

Yup. This is a "one person at a time" process.

This PR does the server side work for adding the Get Index API to the REST high-level-client, namely moving resolving default settings to the transport action. A follow up would be the client side changes.

-- It was disabled by elastic#31675

-- It was disabled by #31675

javanna · 2018-07-04T08:26:03Z

thanks a lot for working on this @sohaibiftikhar , great work!

* 6.x: Fix not waiting for Netty ThreadDeathWatcher in IT (#31758) (#31789) [Docs] Correct default window_size (#31582) S3 fixture should report 404 on unknown bucket (#31782) [ML] Limit ML filter items to 10K (#31731) Fixture for Minio testing (#31688) [ML] Return statistics about forecasts as part of the jobsstats and usage API (#31647) [DOCS] Add missing get mappings docs to HLRC (#31765) [DOCS] Starting Elasticsearch (#31701) Fix coerce validation_method in GeoBoundingBoxQueryBuilder (#31747) Painless: Complete Removal of Painless Type (#31699) Consolidate watcher setting update registration (#31762) [DOCS] Adds empty 6.3.1 release notes page ingest: Introduction of a bytes processor (#31733) [test] don't run bats tests for suse boxes (#31749) Add analyze API to high-level rest client (#31577) Implemented XContent serialisation for GetIndexResponse (#31675) [DOCS] Typos DOC: Add examples to the SQL docs (#31633) Add support for AWS session tokens (#30414) Watcher: Reenable start/stop yaml tests (#31754) JDBC: Fix stackoverflow on getObject and timestamp conversion (#31735) Support multiple system store types (#31650) Add write*Blob option to replace existing blob (#31729) Split CircuitBreaker-related tests (#31659) Painless: Add Context Docs (#31190) Docs: Remove missing reference Migrate scripted metric aggregation scripts to ScriptContext design (#30111) Watcher: Fix chain input toXcontent serialization (#31721) Remove _all example (#31711) rest-high-level: added get cluster settings (#31706) Docs: Match the examples in the description (#31710) [Docs] Correct typos (#31720) Extend allowed characters for grok field names (#21745) (#31653) (#31722) [DOCS] Check for Windows and *nix file paths (#31648) [ML] Validate ML filter_id (#31535) Fix gradle4.8 deprecation warnings (#31654) Update numbers to reflect 4-byte UTF-8-encoded characters (#27083)

* master: [ML] Rate limit established model memory updates (#31768) [Docs] Correct default window_size (#31582) S3 fixture should report 404 on unknown bucket (#31782) Detach Transport from TransportService (#31727) [ML] Limit ML filter items to 10K (#31731) [ML] Return statistics about forecasts as part of the jobsstats and usage API (#31647) Fixture for Minio testing (#31688) [DOCS] Add missing get mappings docs to HLRC (#31765) [DOCS] Starting Elasticsearch (#31701) Painless: Complete Removal of Painless Type (#31699) Fix not waiting for Netty ThreadDeathWatcher in IT (#31758) Consolidate watcher setting update registration (#31762) Build: re-enabled bwc (#31769) ingest: Introduction of a bytes processor (#31733) Fix coerce validation_method in GeoBoundingBoxQueryBuilder (#31747) Add analyze API to high-level rest client (#31577) [DOCS] Typos DOC: Add examples to the SQL docs (#31633) Add support for AWS session tokens (#30414) Watcher: Reenable start/stop yaml tests (#31754) Implemented XContent serialisation for GetIndexResponse (#31675) JDBC: Fix stackoverflow on getObject and timestamp conversion (#31735) resolveHasher defaults to NOOP (#31723) Account for XContent overhead in in-flight breaker Split CircuitBreaker-related tests (#31659) Add write*Blob option to replace existing blob (#31729) Painless: Add Context Docs (#31190) Watcher: Fix chain input toXcontent serialization (#31721) Docs: Match the examples in the description (#31710) rest-high-level: added get cluster settings (#31706) [Docs] Correct typos (#31720) Clean up double semicolon code typos (#31687) [DOCS] Check for Windows and *nix file paths (#31648) [ML] Validate ML filter_id (#31535) Revert long lines Fix TransportChangePasswordActionTests

tvernum added review labels Jun 29, 2018

sohaibiftikhar mentioned this pull request Jun 30, 2018

REST high-level client: add get index API #31703

Merged

1 task

nik9000 self-requested a review July 2, 2018 12:45

nik9000 reviewed Jul 2, 2018

View reviewed changes

nik9000 added >enhancement :Core/Infra/Transport API Transport client API and removed :Core/Java High Level REST Client labels Jul 2, 2018

nik9000 added v7.0.0 v6.4.0 labels Jul 2, 2018

sohaibiftikhar added 4 commits July 2, 2018 23:53

Implemented toXContent and fromXContent for GetIndexResponse

a65bc87

This commit does the all the server side work adding the Get Index API to the REST high-level-client. A follow up would be the client side changes

Added response tests for transport layer serialization

1d8c747

changed hashcode for GetIndexResponse

6c0847b

Switched to v6.4.0

3947a7d

sohaibiftikhar force-pushed the get_index branch from 09b4e96 to 3947a7d Compare July 2, 2018 22:03

disabled bwc check

a300e1d

-- will be reenabled after backport

nik9000 merged commit a5fd4a7 into elastic:master Jul 3, 2018

sohaibiftikhar added a commit to sohaibiftikhar/elasticsearch that referenced this pull request Jul 3, 2018

re-enabled bwc

0ef2156

-- It was disabled by elastic#31675

sohaibiftikhar mentioned this pull request Jul 3, 2018

Re-enable BWC checks #31769

Merged

sohaibiftikhar deleted the get_index branch July 3, 2018 15:26

nik9000 pushed a commit that referenced this pull request Jul 3, 2018

Build: re-enabled bwc (#31769)

dc869aa

-- It was disabled by #31675

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Conversation

sohaibiftikhar commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jun 29, 2018

Uh oh!

tvernum commented Jun 29, 2018

Uh oh!

javanna commented Jun 29, 2018

Uh oh!

nik9000 left a comment

Choose a reason for hiding this comment

Uh oh!

nik9000 Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

sohaibiftikhar Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

nik9000 Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

nik9000 Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

sohaibiftikhar Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

nik9000 Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

sohaibiftikhar Jul 2, 2018

Choose a reason for hiding this comment

Uh oh!

sohaibiftikhar commented Jul 2, 2018

Uh oh!

nik9000 commented Jul 2, 2018

Uh oh!

sohaibiftikhar commented Jul 3, 2018

Uh oh!

nik9000 commented Jul 3, 2018

Uh oh!

javanna commented Jul 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sohaibiftikhar commented Jun 29, 2018 •

edited

Loading