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.

Sign up for GitHub

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

Jump to bottom

[Remove] Types from PutIndexTemplateRequest and builder to reduce mapping to a string #2510

Merged
merged 3 commits into from
Mar 21, 2022
Merged

[Remove] Types from PutIndexTemplateRequest and builder to reduce mapping to a string #2510

merged 3 commits into from
Mar 21, 2022

Conversation

dreamer-89
Copy link
Member

Signed-off-by: Suraj Singh [email protected]

Description

With type removal from codebase, this change prevents PutIndexTemplateRequest and corresponding builder to hold multiple mapping types. It change internal representation of mapping from maps to String and a subsequent change of #2497

Related: #1940

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dreamer-89 dreamer-89 requested a review from a team as a code owner March 18, 2022 18:15
@dreamer-89 dreamer-89 changed the title [Remove] Type from PutIndexTemplateRequest [Remove] Types from PutIndexTemplateRequest and builder to contain a single mapping string Mar 18, 2022
@dreamer-89 dreamer-89 changed the title [Remove] Types from PutIndexTemplateRequest and builder to contain a single mapping string [Remove] Types from PutIndexTemplateRequest and builder to reduce mapping to a string Mar 18, 2022
@dreamer-89
formatting via spotlessApply
956e8ed 
Signed-off-by: Suraj Singh <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 6f0774e
Log 3520

Reports 3520

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 956e8ed
Log 3521

Reports 3521

reta
reta reviewed Mar 18, 2022
View reviewed changes
server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java Outdated
XContentFactory.jsonBuilder()
.startObject()
.startObject("type1")
.startObject("_doc")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the _doc part is not needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @reta for your comment. Yes, it looks like _doc is not needed.

I will remove all occurrences of _doc added as part of this commit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@dreamer-89
Copy link
Member Author

Existing gradle check is failing due to parsing issue. Probable bug in logic change in PutIndexTemplateRequest.toXContent method

  2> REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.action.admin.indices.template.put.PutIndexTemplateRequestTests.testFromXContent" -Dtests.seed=B211A97EE49E7566 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sl-SI -Dtests.timezone=Asia/Aqtau -Druntime.java=17
  2> java.lang.NullPointerException: Cannot invoke "String.length()" because "content" is null
        at __randomizedtesting.SeedInfo.seed([B211A97EE49E7566:93799C666D3995BC]:0)
        at com.fasterxml.jackson.core.JsonFactory.createParser(JsonFactory.java:1158)
        at org.opensearch.common.xcontent.json.JsonXContent.createParser(JsonXContent.java:97)
        at org.opensearch.action.admin.indices.template.put.PutIndexTemplateRequest.toXContent(PutIndexTemplateRequest.java:523)

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure de1f11b371ba0c78b3703e44d5b72886dd1a1323
Log 3526

Reports 3526

@dreamer-89 dreamer-89 force-pushed the remove-type-put-indextemplate branch from de1f11b to 0fa6b43 Compare March 18, 2022 22:42
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0fa6b43
Log 3530

Reports 3530

@dreamer-89
Copy link
Member Author

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0fa6b43
Log 3531

Reports 3531

@dreamer-89
Copy link
Member Author 

**REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=822F6BCFE6BD6C51 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-KW -Dtests.timezone=Asia/Colombo -Druntime.java=17

org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([822F6BCFE6BD6C51:8559BE04DD3B1A0C]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)**

@dreamer-89
Copy link
Member Author

start gradle check

@dreamer-89 dreamer-89 added v2.0.0 Version 2.0.0 >breaking Identifies a breaking change. labels Mar 18, 2022
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 0fa6b43
Log 3532

Reports 3532

@dreamer-89
Copy link
Member Author

@nknize : I would also request your review on this PR as it involves changes around serialisation. Can you please have a look ?

@dreamer-89 dreamer-89 requested a review from nknize March 21, 2022 18:23
@dreamer-89 dreamer-89 added non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues Indexing & Search labels Mar 21, 2022
@dblock dblock requested a review from andrross March 21, 2022 20:13
nknize
nknize approved these changes Mar 21, 2022
View reviewed changes
Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

LGTM!

@nknize nknize merged commit 59d1e69 into opensearch-project:main Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nknize nknize nknize approved these changes

@reta reta reta approved these changes

@andrross andrross Awaiting requested review from andrross andrross is a code owner

Assignees
No one assigned
Labels
>breaking Identifies a breaking change. Indexing & Search non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues v2.0.0 Version 2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dreamer-89 @opensearch-ci-bot @reta @nknize