[Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3346

reta · 2022-05-16T20:38:15Z

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Description

There are number of deprecated methods in JodaCompatibleZonedDateTime which are still available in scripts. Those should be removed since in some cases they pose significant performance overhead (due to deprecation warnings).

Issues Resolved

Closes #3156 and #3345

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

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.

opensearch-ci-bot · 2022-05-16T20:48:37Z

❌ Gradle Check failure 68f49f4f93b2df18c7c805aeb8e1ce5f130c423e
Log 5378

Reports 5378

saratvemulapalli · 2022-05-16T20:50:34Z

server/src/main/java/org/opensearch/script/JodaCompatibleZonedDateTime.java

@@ -427,120 +401,7 @@ public ZonedDateTime withZoneSameInstant(ZoneId zone) {
        return dt.withZoneSameInstant(zone);
    }

-    @Deprecated


Are you also looking at backporting this to 2.0?
If not, can we start curating a breaking changes meta issue for 3.0?
This is a breaking change and there could be plugins in the wild using these methods.

Yes, sorry, forgot to add the labels.

saratvemulapalli · 2022-05-16T20:51:09Z

start gradle check

saratvemulapalli · 2022-05-16T20:52:48Z

@reta precommit is failing because of a forbidden API:

Forbidden method invocation: java.time.format.DateTimeFormatter#ofPattern(java.lang.String) [Uses default locale]

  in org.opensearch.script.JodaCompatibleZonedDateTimeTests (JodaCompatibleZonedDateTimeTests.java:191)

opensearch-ci-bot · 2022-05-16T20:56:51Z

❌ Gradle Check failure 68f49f4f93b2df18c7c805aeb8e1ce5f130c423e
Log 5379

Reports 5379

opensearch-ci-bot · 2022-05-16T21:04:49Z

❌ Gradle Check failure 1562f9ff898feb42ff7cf4f294035fc81d10d1d9
Log 5380

Reports 5380

…called by scripts Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

opensearch-ci-bot · 2022-05-16T21:46:25Z

✅ Gradle Check success 403b947
Log 5381

Reports 5381

…called by scripts (#3346) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit c470580)

…called by scripts (#3346) (#3347) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit c470580) Co-authored-by: Andriy Redko <andriy.redko@aiven.io>

…called by scripts (#3346) (#3348) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit c470580) Co-authored-by: Andriy Redko <andriy.redko@aiven.io>

kavilla · 2022-05-18T22:22:13Z

I believe this might be a large breaking change to get merged into 2.0. OpenSearch Dashboards and visualizes utilizes painless scripts. I did not expect that we were removing this for 2.0 so a lot of tests are failing and I can see others who have their painless scripts defined will need to ensure update them prior to migrating to OpenSearch 2.0.

It's okay but I think this is a huge impact to the community and even I was caught by surprise even though I'm a maintainer on OpenSearch Dashboards. Maybe I missed this heads up but was this called out a head of time?

saratvemulapalli · 2022-05-18T22:50:00Z

@kavilla its fair. Ideally deprecated method usage should have been migrated. But we agree it was not the right practice we removed it after launching RC1. It should have been in the RC1 but we missed the train.
We do document all the breaking changes in #2480 and via >breaking label. If you have other suggestions to notify consumers in a better way, we'd love to hear.

@reta I'll go revert this in 2.0, 2.x line and let it go out in 3.x to unblock the build for 2.0 release.
Let me know if you strongly feel otherwise.

I've removed the entry in 2.0 breaking changes(#2480) and added this into 3.0 (#3351)

…ich are called by scripts (#3346) (#3347)" (#3394) This reverts commit 3537b5a. Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>

…ich are called by scripts (#3346) (#3348)" (#3393) This reverts commit a764fde. Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>

reta · 2022-05-19T04:14:11Z

@saratvemulapalli sounds good, thank you, sorry for that @kavilla

reta · 2022-05-19T04:20:07Z

@kavilla I will create an issue on OpenSearch Dashboards, the deprecated methods introduce significant overhead because of the deprecation logger (vs actual useful work the methods do), we have to migrate the scripts to use the non-deprecated variants.

Sample data and functional tests were utilizing deprecated methods for painless scripts. However these deprecated methods were removed in: opensearch-project/OpenSearch#3346 This is to ensure sample data and ftr tests work but end users saved objects might contain painless scripts that we might not be able to address. So we should research if this it is possible to help mitigate this. Related issue: opensearch-project#1600 Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* [Build][Tests] handle painless scripts update Sample data and functional tests were utilizing deprecated methods for painless scripts. However these deprecated methods were removed in: opensearch-project/OpenSearch#3346 This is to ensure sample data and ftr tests work but end users saved objects might contain painless scripts that we might not be able to address. So we should research if this it is possible to help mitigate this. Related issue: #1600 Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* [Build][Tests] handle painless scripts update Sample data and functional tests were utilizing deprecated methods for painless scripts. However these deprecated methods were removed in: opensearch-project/OpenSearch#3346 This is to ensure sample data and ftr tests work but end users saved objects might contain painless scripts that we might not be able to address. So we should research if this it is possible to help mitigate this. Related issue: opensearch-project#1600 Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

* Revert "Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts (#3346) (#3347)" This reverts commit 3537b5a. Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com> * Bumping 2.x to 2.2 Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com> * Updating lucene verion for 2.1 Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>

* [Build][Tests] handle painless scripts update Sample data and functional tests were utilizing deprecated methods for painless scripts. However these deprecated methods were removed in: opensearch-project/OpenSearch#3346 This is to ensure sample data and ftr tests work but end users saved objects might contain painless scripts that we might not be able to address. So we should research if this it is possible to help mitigate this. Related issue: opensearch-project#1600 Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

reta requested a review from a team as a code owner May 16, 2022 20:38

saratvemulapalli approved these changes May 16, 2022

View reviewed changes

reta force-pushed the issue-3345 branch from 68f49f4 to 1562f9f Compare May 16, 2022 20:54

reta added backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch labels May 16, 2022

reta mentioned this pull request May 16, 2022

[Documentation] Breaking changes in 2.0 #2480

Closed

10 tasks

reta changed the title ~~Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts~~ [Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts May 16, 2022

Remove deprecated methods from JodaCompatibleZonedDateTime which are …

403b947

…called by scripts Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta force-pushed the issue-3345 branch from 1562f9f to 403b947 Compare May 16, 2022 21:14

saratvemulapalli added >breaking Identifies a breaking change. v3.0.0 Issues and PRs related to version 3.0.0 v2.0.0 Version 2.0.0 labels May 16, 2022

saratvemulapalli merged commit c470580 into opensearch-project:main May 16, 2022

opensearch-trigger-bot bot pushed a commit that referenced this pull request May 16, 2022

Remove deprecated methods from JodaCompatibleZonedDateTime which are …

4b20528

…called by scripts (#3346) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit c470580)

opensearch-trigger-bot bot mentioned this pull request May 16, 2022

[Backport 2.x] [Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3347

Merged

opensearch-trigger-bot bot pushed a commit that referenced this pull request May 16, 2022

Remove deprecated methods from JodaCompatibleZonedDateTime which are …

c1a53b2

…called by scripts (#3346) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit c470580)

opensearch-trigger-bot bot mentioned this pull request May 16, 2022

[Backport 2.0] [Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3348

Merged

This was referenced May 18, 2022

[2.0] Revert "Remove deprecated methods from JodaCompatibleZonedDateTime wh… #3393

Merged

[META] Breaking changes in 3.0 #3351

Open

kavilla mentioned this pull request May 19, 2022

[Build][Tests] handle painless scripts update opensearch-project/OpenSearch-Dashboards#1607

Merged

7 tasks

This was referenced May 23, 2022

[FEATURE] Handle deprecations better opensearch-project/OpenSearch-Dashboards#1610

Open

[BUG] login failed: Invalid username or password, please try again opensearch-project/OpenSearch-Dashboards#1611

Open

saratvemulapalli mentioned this pull request Jul 18, 2022

Upgrading Joda version #3935

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3346

[Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3346

reta commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

saratvemulapalli May 16, 2022

reta May 16, 2022

saratvemulapalli commented May 16, 2022

saratvemulapalli commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

kavilla commented May 18, 2022 •

edited

Loading

saratvemulapalli commented May 18, 2022 •

edited

Loading

reta commented May 19, 2022 •

edited

Loading

reta commented May 19, 2022

[Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3346

[Remove] Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts #3346

Conversation

reta commented May 16, 2022

Description

Issues Resolved

Check List

opensearch-ci-bot commented May 16, 2022

saratvemulapalli May 16, 2022

Choose a reason for hiding this comment

reta May 16, 2022

Choose a reason for hiding this comment

saratvemulapalli commented May 16, 2022

saratvemulapalli commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

opensearch-ci-bot commented May 16, 2022

kavilla commented May 18, 2022 • edited Loading

saratvemulapalli commented May 18, 2022 • edited Loading

reta commented May 19, 2022 • edited Loading

reta commented May 19, 2022

kavilla commented May 18, 2022 •

edited

Loading

saratvemulapalli commented May 18, 2022 •

edited

Loading

reta commented May 19, 2022 •

edited

Loading