refactor: create AbstractAggregationRequestIT class + fail with OpenS… #421

szczepanczykd · 2023-03-30T19:54:43Z

…earchException details if thrown (#407)

Description

Let's see why the tests are failing.

Changes:

explicit mappings for test indices
change expDate from Date to String (seems string date is more stable)
create AbstractAggregationRequestIT ( AggregationRequestIT no longer extends AbstractRequestIT)
save tests reports on failed workflow -> now we can see more details

Issues Resolved

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.

…earchException details if thrown (opensearch-project#407) Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

reta · 2023-03-30T20:39:34Z

org.opensearch.client.opensearch.integTest.httpclient5.AggregationRequestIT > testDateRangeAggregation FAILED
    java.lang.AssertionError at [FA4AE380E09F6D26:DEE1BC2339169740]:0

org.opensearch.client.opensearch.integTest.restclient.AggregationRequestIT > testDateRangeAggregation FAILED
    java.lang.AssertionError at [7675EA169D5AC8:24DD2A49CF14A0AE]:0

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

szczepanczykd · 2023-03-30T20:44:28Z

org.opensearch.client.opensearch.integTest.httpclient5.AggregationRequestIT > testDateRangeAggregation FAILED
    java.lang.AssertionError at [FA4AE380E09F6D26:DEE1BC2339169740]:0

org.opensearch.client.opensearch.integTest.restclient.AggregationRequestIT > testDateRangeAggregation FAILED
    java.lang.AssertionError at [7675EA169D5AC8:24DD2A49CF14A0AE]:0

Yeah, it tells nothing :|
We have to dig into the test logs.

Logs:
java.lang.AssertionError: Exception while searching: index: test-date-range-aggregation ErrorMsg: Request failed: [search_phase_execution_exception] all shards failed status: 400 reason: all shards failed type: search_phase_execution_exception

More logs:

java.lang.AssertionError: Exception while searching: index: test-date-range-aggregation
 ErrorMsg: Request failed: [search_phase_execution_exception] all shards failed
 status: 400
 error: reason: all shards failed
type: search_phase_execution_exception
meta: {phase="query", failed_shards=[{"shard":0,"index":"test-date-range-aggregation","node":"OTr04M9VQv2dRwtIVvLL4A","reason":{"type":"illegal_argument_exception","reason":"Text fields are not optimised for operations that require per-document field data like aggregations and sorting, so these operations are disabled by default. Please use a keyword field instead. Alternatively, set fielddata=true on [expDate] in order to load field data by uninverting the inverted index. Note that this can use significant memory."}}], grouped=true}

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

szczepanczykd · 2023-03-31T08:39:47Z

@VachaShah @reta - please take a look

reta · 2023-03-31T13:20:51Z

.github/workflows/test-integration-unreleased.yml

@@ -55,3 +55,11 @@ jobs:
        run: |
          cd opensearch-java
          ./gradlew clean integrationTest -Dhttps=false
+


reta · 2023-03-31T13:21:18Z

.github/workflows/test-integration-unreleased.yml

+        with:
+          name: test-reports
+          path: opensearch-java/java-client/build/reports/
+          retention-days: 30


Probably 30 days is a lot, 7 days? (a week)

The default value is 90 ->
updated to 7.

Should be enough to read the report :D

reta · 2023-03-31T13:25:32Z

...t/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAggregationRequestIT.java

@@ -190,13 +189,12 @@ private String getErrorMetadata(ErrorCause errorCause) {
 	public static class ProductDetails {
 		private String name;
 		private int cost;
-		@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss.SSS")
-		private Date expDate;
+		private String expDate;


This change is not very straightforward: definitely 👍 to move off the Date, but using string to pass the dates through is not a best option I think:

should we use LocateDateTime / Instant instead` ?

should be use the timestamp value (long)?

String allows to insert date in ISO format without any transformations.

LocateDateTime and Instant are requiring:

@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ssZ", timezone = "UTC") private Instant expDate;

build.gradle.kts:
testImplementation("com.fasterxml.jackson.datatype", "jackson-datatype-jsr310", jacksonVersion)

JacksonJsonpMapper -> findAndRegisterModules method used:

new ObjectMapper() .findAndRegisterModules() .configure(SerializationFeature.INDENT_OUTPUT, false) .setSerializationInclusion(JsonInclude.Include.NON_NULL)

private Long expDate; does not require anything more.
Seems it works but the minus is that the Document is created with numeric values:

"_index": "test-date-range-aggregation", "_type": "_doc", "_id": "1", "_score": 1, "_source": { "name": "egg", "cost": 2, "expDate": 1676937600000 } }

So I'm not sure what is better than simple String ;P

String allows to insert date in ISO format without any transformations.

That is what `@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = ISO) is for, piratically this is how the feature should be used (yes, using string does simplify tests).

private Long expDate; does not require anything more.

That's fine, the OS is able to deal with date in different formats.

Ok, let's stick with Long expDate.

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

opensearch-trigger-bot · 2023-04-03T17:59:24Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-421-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 765c80351113dae0650db15d3f7c254f2673e112
# Push it to GitHub
git push --set-upstream origin backport/backport-421-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-421-to-2.x.

dblock · 2023-04-03T18:02:30Z

@szczepanczykd I think we want this on the 2.x branch, care to backport manually?

szczepanczykd · 2023-04-03T18:20:47Z

@daniel D. ***@***.***>I will try tomorrow :)

…

On Mon, 3 Apr 2023, 20:02 Daniel (dB.) Doubrovkine, < ***@***.***> wrote: @szczepanczykd <https://github.com/szczepanczykd> I think we want this on the 2.x branch, care to backport manually? — Reply to this email directly, view it on GitHub <#421 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALVQM4NOZCTQZZCNUFYWVF3W7MGEFANCNFSM6AAAAAAWNYAYVE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

opensearch-project#421) * refactor: create AbstractAggregationRequestIT class + fail with OpenSearchException details if thrown (opensearch-project#407) Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * feature: upload IntegrationTests reports Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * fix: fixed workflow file Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * fix: upload reports on failure Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: add more error details to tests Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * feat: update github workflows with name and retention-days Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * test: create index with manual mappings Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: change expDate type from Date to String Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: stability check Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: stability check 2 Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * fix: change retention-days to 7 for uploaded artifacts Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: change expDate type to Long Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> --------- Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> (cherry picked from commit 765c803)

#436) * refactor: create AbstractAggregationRequestIT class + fail with OpenS… (#421) * refactor: create AbstractAggregationRequestIT class + fail with OpenSearchException details if thrown (#407) Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * feature: upload IntegrationTests reports Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * fix: fixed workflow file Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * fix: upload reports on failure Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: add more error details to tests Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * feat: update github workflows with name and retention-days Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * test: create index with manual mappings Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: change expDate type from Date to String Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: stability check Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: stability check 2 Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * fix: change retention-days to 7 for uploaded artifacts Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> * refactor: change expDate type to Long Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> --------- Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> (cherry picked from commit 765c803) * fix: use HttpHost from apache package Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com> --------- Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

refactor: create AbstractAggregationRequestIT class + fail with OpenS…

65ed8c3

…earchException details if thrown (opensearch-project#407) Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

szczepanczykd requested review from reta, Bukhtawar, dblock, madhusudhankonda, saratvemulapalli and VachaShah as code owners March 30, 2023 19:54

reta added the skip-changelog label Mar 30, 2023

feature: upload IntegrationTests reports

d938d19

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

szczepanczykd added 8 commits March 30, 2023 22:47

fix: fixed workflow file

bf9b658

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

fix: upload reports on failure

51e0821

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

refactor: add more error details to tests

11210c4

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

feat: update github workflows with name and retention-days

e168d8d

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

test: create index with manual mappings

5ee1c48

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

refactor: change expDate type from Date to String

986d6dc

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

refactor: stability check

71944df

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

refactor: stability check 2

c8f6e5e

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

reta reviewed Mar 31, 2023

View reviewed changes

szczepanczykd added 2 commits March 31, 2023 15:31

fix: change retention-days to 7 for uploaded artifacts

26753a9

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

refactor: change expDate type to Long

bc4eaec

Signed-off-by: Dominik Szczepanczyk <szczepanczyk.dominik@gmail.com>

reta approved these changes Mar 31, 2023

View reviewed changes

reta mentioned this pull request Apr 3, 2023

Feature/get missing alias json exception 301 #399

Closed

dblock approved these changes Apr 3, 2023

View reviewed changes

dblock merged commit 765c803 into opensearch-project:main Apr 3, 2023

dblock added the backport 2.x Backport to 2.x branch label Apr 3, 2023

szczepanczykd mentioned this pull request Apr 5, 2023

refactor: create AbstractAggregationRequestIT class + fail with OpenS… #435

Closed

szczepanczykd mentioned this pull request Apr 5, 2023

refactor: create AbstractAggregationRequestIT class + fail with OpenS… #436

Merged

VachaShah mentioned this pull request Apr 11, 2023

[BUG] Flaky test org.opensearch.client.opensearch.integTest.restclient.AggregationRequestIT.testDateRangeAggregation #407

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: create AbstractAggregationRequestIT class + fail with OpenS… #421

refactor: create AbstractAggregationRequestIT class + fail with OpenS… #421

szczepanczykd commented Mar 30, 2023 •

edited

Loading

reta commented Mar 30, 2023

szczepanczykd commented Mar 30, 2023 •

edited

Loading

szczepanczykd commented Mar 31, 2023

reta Mar 31, 2023

reta Mar 31, 2023

szczepanczykd Mar 31, 2023

reta Mar 31, 2023

szczepanczykd Mar 31, 2023

reta Mar 31, 2023 •

edited

Loading

szczepanczykd Mar 31, 2023

opensearch-trigger-bot bot commented Apr 3, 2023

dblock commented Apr 3, 2023

szczepanczykd commented Apr 3, 2023 via email

refactor: create AbstractAggregationRequestIT class + fail with OpenS… #421

refactor: create AbstractAggregationRequestIT class + fail with OpenS… #421

Conversation

szczepanczykd commented Mar 30, 2023 • edited Loading

Description

Issues Resolved

reta commented Mar 30, 2023

szczepanczykd commented Mar 30, 2023 • edited Loading

szczepanczykd commented Mar 31, 2023

reta Mar 31, 2023

Choose a reason for hiding this comment

reta Mar 31, 2023

Choose a reason for hiding this comment

szczepanczykd Mar 31, 2023

Choose a reason for hiding this comment

reta Mar 31, 2023

Choose a reason for hiding this comment

szczepanczykd Mar 31, 2023

Choose a reason for hiding this comment

reta Mar 31, 2023 • edited Loading

Choose a reason for hiding this comment

szczepanczykd Mar 31, 2023

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Apr 3, 2023

dblock commented Apr 3, 2023

szczepanczykd commented Apr 3, 2023 via email

szczepanczykd commented Mar 30, 2023 •

edited

Loading

szczepanczykd commented Mar 30, 2023 •

edited

Loading

reta Mar 31, 2023 •

edited

Loading