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] type from CIR.mapping and CIRB.mapping #2478

Merged
merged 6 commits into from
Mar 16, 2022
Merged

[Remove] type from CIR.mapping and CIRB.mapping #2478

merged 6 commits into from
Mar 16, 2022

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Mar 15, 2022

First pass to remove types from CreateIndexRequest and CreateIndexRequestBuilder
mapping method. This method is overloaded several times so the most widely used
methods in the RequestBuilder are refactored from mapping to setMapping to avoid
confusion, conflicts, and to be consistent with other method names (e.g.,
setSettings, setCause, setAlias).

relates #1940

@nknize nknize added v2.0.0 Version 2.0.0 >breaking Identifies a breaking change. Indexing & Search labels Mar 15, 2022
@nknize nknize requested a review from a team as a code owner March 15, 2022 18:12
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure ce3242eafca12e888ebd1398069a75ced2bdc989
Log 3413

Reports 3413

@nknize nknize force-pushed the remove/typeFromCIRMapping1 branch from ce3242e to 7b36c28 Compare March 15, 2022 20:02
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 7b36c2834d1888d5ce2fa4158ae50de7ec71d9c7
Log 3418

Reports 3418

@dreamer-89
Copy link
Member

dreamer-89 commented Mar 15, 2022 •
edited
Loading

@nknize : In gradle check failure, I see similar errors as we were getting in #2460. Does it need rebase against main ?

@nknize
Copy link
Collaborator Author

nknize commented Mar 15, 2022

Does it need rebase against main ?

Yes. I'll definitely need to rebase now that the lucene 9 PR has landed.

@nknize nknize force-pushed the remove/typeFromCIRMapping1 branch from 7b36c28 to 0e06bdd Compare March 15, 2022 21:14
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0e06bdd387da328eabd3720054cf729354b334a5
Log 3423

Reports 3423

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 48208afd67e2248e96c4364511866a05e17e1058
Log 3427

Reports 3427

@nknize nknize force-pushed the remove/typeFromCIRMapping1 branch from 3e1010e to 776f152 Compare March 16, 2022 16:43
@dreamer-89
Copy link
Member

Please ignore my previous comments, will wait for current gradle check to complete before review.

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 776f15239f94a2e5643c1dffee71f598a7ea304d
Log 3452

Reports 3452

dreamer-89
dreamer-89 approved these changes Mar 16, 2022
View reviewed changes
Copy link
Member

@dreamer-89 dreamer-89 left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

server/src/main/java/org/opensearch/action/admin/indices/create/CreateIndexRequest.java
Objects.requireNonNull(xContentType);
Map<String, Object> mappingAsMap = XContentHelper.convertToMap(source, false, xContentType).v2();
return mapping(type, mappingAsMap);
return mapping(MapperService.SINGLE_MAPPING_NAME, mappingAsMap);
Copy link
Member

Choose a reason for hiding this comment

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

With type removal, do we still need method private CreateIndexRequest mapping(String type, Map<String, ?> source) which has type related checks/validations ?

If this is not needed, I am fine removing this in follow up PR.

Copy link
Collaborator Author

@nknize nknize Mar 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This will be removed in a followup, just to keep the blast radius "small" for now

test/framework/src/main/java/org/opensearch/index/RandomCreateIndexGenerator.java Outdated Show resolved Hide resolved
@nknize
[Remove] type from CIR.mapping and CIRB.mapping
b1bd13b 
First pass to remove types from CreateIndexRequest and CreateIndexRequestBuilder
mapping method. This method is overloaded several times so the most widely used
methods in the RequestBuilder are refactored from mapping to setMapping to avoid
confusion, conflicts, and to be consistent with other method names (e.g.,
setSettings, setCause, setAlias).

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
test fixes
ac825f6 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
more test fixes
24d5e02 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
more test fixes
e4159cb 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
more test fixes removing types
038e004 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
update doc
d40271f 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize nknize force-pushed the remove/typeFromCIRMapping1 branch from 776f152 to d40271f Compare March 16, 2022 19:24
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success d40271f
Log 3455

Reports 3455

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

@dreamer-89 dreamer-89 dreamer-89 approved these changes

Assignees
No one assigned
Labels
>breaking Identifies a breaking change. Indexing & Search v2.0.0 Version 2.0.0
Projects
None yet
Milestone
No milestone
Development

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