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

Fix create_index/create_index_with_IOException issue caused by OpenSearch PR change #1899

Merged
merged 9 commits into from
Jul 26, 2023
Merged

Fix create_index/create_index_with_IOException issue caused by OpenSearch PR change #1899

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
843cf0a
Applying Google Java code format changes to core/src/main/java/org/op…
MitchellGale Jul 25, 2023
7696251
Added setDefaultMediaType for create_index and create_index_with_IOEx…
MitchellGale Jul 25, 2023
f61286c
Revert "Applying Google Java code format changes to core/src/main/jav…
MitchellGale Jul 25, 2023
d6c445f
Moved set default media to beforeall
MitchellGale Jul 25, 2023
5a19575
Removed type comment
MitchellGale Jul 25, 2023
3b9f5f3
Removed wildcard import.
MitchellGale Jul 25, 2023
2f4b8f7
Removed wildcard import.
MitchellGale Jul 25, 2023
8b0e8af
added import for before all and before each
MitchellGale Jul 25, 2023
a2a588e
Fixed import order for OpenSearchRestClientTest.java
MitchellGale Jul 25, 2023
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
4 changes: 3 additions & 1 deletion opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.opensearch.core.xcontent.MediaTypeParserRegistry.setDefaultMediaType;
import static org.opensearch.sql.opensearch.client.OpenSearchClient.META_CLUSTER_NAME;
import static org.opensearch.sql.opensearch.data.type.OpenSearchDataType.MappingType;

Expand Down Expand Up @@ -129,6 +130,7 @@ void is_index_exist_with_exception() throws IOException {
@Test
void create_index() throws IOException {
String indexName = "test";
setDefaultMediaType(XContentType.JSON);
Copy link
Member

Choose a reason for hiding this comment

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

Is this expected? How does this work when a new cluster is deployed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Before 8636 core used JSON hardcoded. Now it uses DefaultMediaType which is unset (null) by default.
You can re-run these tests on a branch to see the failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the setDefaultMediaType to a BeforeAll. but yes I believe it is the expected Media Type for these tests.

Copy link
Member

Choose a reason for hiding this comment

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

@Yury-Fridlyand I looked into this yesterday, I couldn't figure out the need to introduce a new function for setting default type in core. Probably I should comment in 8636.

Map<String, Object> mappings = ImmutableMap.of(
"properties",
ImmutableMap.of("name", "text"));
Expand All @@ -142,7 +144,7 @@ void create_index() throws IOException {
@Test
void create_index_with_IOException() throws IOException {
when(restClient.indices().create(any(), any())).thenThrow(IOException.class);

setDefaultMediaType(XContentType.JSON);
assertThrows(IllegalStateException.class,
() -> client.createIndex("test", ImmutableMap.of()));
}
Expand Down
Loading