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

Deprecate Map, List, and Set in org.opensearch.common.collect (#6609)… #6687

Merged
merged 1 commit into from
Mar 16, 2023
Merged

Deprecate Map, List, and Set in org.opensearch.common.collect (#6609)… #6687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Refactor] XContent base classes from xcontent to core library ([#5902](https://github.com/opensearch-project/OpenSearch/pull/5902))

### Deprecated
- Map, List, and Set in org.opensearch.common.collect ([#6609](https://github.com/opensearch-project/OpenSearch/pull/6609))
Copy link
Member

Choose a reason for hiding this comment

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

This will be an invasive change across the project. Most plugins use these data structures[1].
I would take this opportunity to remove these implementations completely in 3.0 and have these deprecations in 2.x. Since we have the flexibility to make breaking changes in a new major. WDYT @reta @andrross @nknize

[1] https://github.com/search?q=org%3Aopensearch-project%20org.opensearch.common.collect&type=code

Copy link
Collaborator

@reta reta Mar 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@saratvemulapalli it makes sense but the only concern I have is how far are we from 3.0.0 release? We have to give people (plugin developers primarily) enough time to move off the deprecated APIs, if 3.0.0 is year from now - this is fine to deprecate + remove, if it is just few months - we should postpone removal to 4.0.0 I think.

Copy link
Member Author

@kotwanikunal kotwanikunal Mar 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

These deprecations have already been merged as a part of 2.x. I am trying to get main in sync with those changes.
We can have a campaign for removal as a part of 3.0.0, if necessary. But I still think we need to merge this in to get the branches in sync first.

Copy link
Member

@saratvemulapalli saratvemulapalli Mar 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@reta from release schedule[1], looks like we are atleast 9-11 months away from 3.0.
@kotwanikunal absolutely, lets get this merged.

If all 3/other maintainers agree, could you create a campaign that these data structures will be removed?
Please Vote 👍🏻 👎🏻

[1] https://opensearch.org/releases.html

Copy link
Member Author

Choose a reason for hiding this comment

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

I will keep a track on the votes, but as per the current majority, I will create a campaign soon for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Campaign: #6786


### Removed

Expand Down
10 changes: 3 additions & 7 deletions ...nsearch/benchmark/search/aggregations/bucket/terms/StringTermsSerializationBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@
@State(Scope.Benchmark)
public class StringTermsSerializationBenchmark {
private static final NamedWriteableRegistry REGISTRY = new NamedWriteableRegistry(
org.opensearch.common.collect.List.of(
new NamedWriteableRegistry.Entry(InternalAggregation.class, StringTerms.NAME, StringTerms::new)
)
List.of(new NamedWriteableRegistry.Entry(InternalAggregation.class, StringTerms.NAME, StringTerms::new))
);
@Param(value = { "1000" })
private int buckets;
Expand All @@ -75,15 +73,13 @@ public class StringTermsSerializationBenchmark {

@Setup
public void initResults() {
results = DelayableWriteable.referencing(InternalAggregations.from(org.opensearch.common.collect.List.of(newTerms(true))));
results = DelayableWriteable.referencing(InternalAggregations.from(List.of(newTerms(true))));
}

private StringTerms newTerms(boolean withNested) {
List<StringTerms.Bucket> resultBuckets = new ArrayList<>(buckets);
for (int i = 0; i < buckets; i++) {
InternalAggregations inner = withNested
? InternalAggregations.from(org.opensearch.common.collect.List.of(newTerms(false)))
: InternalAggregations.EMPTY;
InternalAggregations inner = withNested ? InternalAggregations.from(List.of(newTerms(false))) : InternalAggregations.EMPTY;
resultBuckets.add(new StringTerms.Bucket(new BytesRef("test" + i), i, inner, false, 0, DocValueFormat.RAW));
}
return new StringTerms(
Expand Down
6 changes: 3 additions & 3 deletions client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2031,8 +2031,8 @@ public void testSimulateIndexTemplate() throws Exception {
Settings settings = Settings.builder().put("index.number_of_shards", 1).build();
CompressedXContent mappings = new CompressedXContent("{\"properties\":{\"host_name\":{\"type\":\"keyword\"}}}");
AliasMetadata alias = AliasMetadata.builder("alias").writeIndex(true).build();
Template template = new Template(settings, mappings, org.opensearch.common.collect.Map.of("alias", alias));
List<String> pattern = org.opensearch.common.collect.List.of("pattern");
Template template = new Template(settings, mappings, Map.of("alias", alias));
List<String> pattern = List.of("pattern");
ComposableIndexTemplate indexTemplate = new ComposableIndexTemplate(
pattern,
template,
Expand All @@ -2057,7 +2057,7 @@ public void testSimulateIndexTemplate() throws Exception {
AliasMetadata simulationAlias = AliasMetadata.builder("simulation-alias").writeIndex(true).build();
ComposableIndexTemplate simulationTemplate = new ComposableIndexTemplate(
pattern,
new Template(null, null, org.opensearch.common.collect.Map.of("simulation-alias", simulationAlias)),
new Template(null, null, Map.of("simulation-alias", simulationAlias)),
Collections.emptyList(),
2L,
1L,
Expand Down
2 changes: 1 addition & 1 deletion libs/common/src/main/java/org/opensearch/common/collect/List.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
/**
* Java 9 List
*
* todo: deprecate and remove w/ min jdk upgrade to 11?
*
* @opensearch.internal
*/
@Deprecated(forRemoval = true)
public class List {

/**
Expand Down
2 changes: 1 addition & 1 deletion libs/common/src/main/java/org/opensearch/common/collect/Map.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
/**
* Java 9 Map
*
* todo: deprecate and remove w/ min jdk upgrade to 11?
*
* @opensearch.internal
*/
@Deprecated(forRemoval = true)
public class Map {

/**
Expand Down
2 changes: 1 addition & 1 deletion libs/common/src/main/java/org/opensearch/common/collect/Set.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
/**
* Java 9 Set
*
* todo: deprecate and remove w/ min jdk upgrade to 11?
*
* @opensearch.internal
*/
@Deprecated(forRemoval = true)
public class Set {

/**
Expand Down
83 changes: 0 additions & 83 deletions libs/common/src/test/java/org/opensearch/common/collect/ListTests.java
View file
Open in desktop

This file was deleted.

210 changes: 0 additions & 210 deletions libs/common/src/test/java/org/opensearch/common/collect/MapTests.java
View file
Open in desktop

This file was deleted.

Loading