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.

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

[Build][Tests] handle painless scripts update #1607

Merged

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented May 19, 2022

Description

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.

Signed-off-by: Kawika Avilla kavilla414@gmail.com

Issues Related

#1600

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

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>
@kavilla kavilla requested a review from a team as a code owner May 19, 2022 21:43
kavilla added 2 commits May 19, 2022 23:05
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Copy link
Contributor

@tmarkley tmarkley left a comment

Choose a reason for hiding this comment

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

LGTM!

How did you find these instances of the deprecated methods? Was there a particular error? Did you have to search across the code base for each one?

@reta
Copy link

reta commented May 20, 2022

Thanks @kavilla !

@kavilla
Copy link
Member Author

kavilla commented May 20, 2022

LGTM!

How did you find these instances of the deprecated methods? Was there a particular error? Did you have to search across the code base for each one?

Pretty much. But for the sample data, pretty straight forward. Just checked the index patterns for the scripted fields. Now I wonder how can we best deliver this information to 3.0 users when they upgrade.

@reta
Copy link

reta commented May 20, 2022

LGTM!
How did you find these instances of the deprecated methods? Was there a particular error? Did you have to search across the code base for each one?

Pretty much. But for the sample data, pretty straight forward. Just checked the index patterns for the scripted fields. Now I wonder how can we best deliver this information to 3.0 users when they upgrade.

Release Notes with noting the breaking changes?

Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

what errors would we see? for example, during oncall, how can we know it is related to getMillis() issue?

@kavilla
Copy link
Member Author

kavilla commented May 20, 2022

LGTM

what errors would we see? for example, during oncall, how can we know it is related to getMillis() issue?

Right now it comes back as a general search_phase exception. It's easier to see on the Discover page because the popup complains with it not knowing what this function is. If you pull down main currently, run yarn opensearch snapshot and add sample flights data and navigate to the Discover page you will see the exception.

But given that is just sample data and anyone can create their scripted fields to be used by visualizations it might not be complete transparent for us in 3.0 right away. It might require telling end users to check their index patterns and verifying their scripts are up to date.

@kavilla
Copy link
Member Author

kavilla commented May 20, 2022

LGTM!
How did you find these instances of the deprecated methods? Was there a particular error? Did you have to search across the code base for each one?

Pretty much. But for the sample data, pretty straight forward. Just checked the index patterns for the scripted fields. Now I wonder how can we best deliver this information to 3.0 users when they upgrade.

Release Notes with noting the breaking changes?

True that will be great on the OpenSearch side but for OpenSearch Dashboards it wouldn't be breaking change from us but it would appear that it's something breaking on our side. Plus we can probably do something in application to warn people ahead of time.

Will tag UI/UX with to see if they have an opinions.

@kavilla kavilla merged commit de1a689 into opensearch-project:main May 20, 2022
@kavilla kavilla deleted the avillk/handle_dep_painless_func branch May 20, 2022 20:02
@joshuarrrr
Copy link
Member

A migration guide would be super useful - given that the scripts here were likely more basic than what many other folks have, we'd probably want to identify a more complex/realistic painless script to use as the example. If we had a migration guide on the docs site, it would also be easy to link to from some sort of helpful UI error message.

kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Jun 8, 2022
* [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>
kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Jun 16, 2022
* [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>
pjfitzgibbons pushed a commit to pjfitzgibbons/OpenSearch-Dashboards that referenced this pull request Aug 10, 2022
* [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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants