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

Changing select index error message to be more informative (#173) #270

Merged

Conversation

evankielley
Copy link
Contributor

@evankielley evankielley commented Sep 1, 2023

Description

When a user submits a search query with no index or only one index selected, an error message is displayed that tells the user to select an index. This pull request changes that error message to be more informative.

Issues Resolved

Resolves #173

Screenshots

Before

(Before) Both left and right indices selected
before-both-selected

(Before) Only left index selected
before-left-selected

(Before) Only right index selected
before-right-selected

(Before) Neither left nor right indices selected
before-none-selected

After

(After) Both left and right indices selected
after-both-selected

(After) Only left index selected
after-left-selected

(After) Only right index selected
after-right-selected

(After) Neither left nor right indices selected
after-none-selected

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.

@noCharger
Copy link
Collaborator

@evankielley Thank you for your contribution to the DSR plugin! Could please add a screenshot of the page before and after the changes. Also, the identical error message appears in multiple files; can we make it a constant? Please disregard the yarn.lock file change. The maintainers are addressing it now.

@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Merging #270 (b18f4ad) into main (77124d5) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #270   +/-   ##
=======================================
  Coverage   86.27%   86.27%           
=======================================
  Files          15       15           
  Lines         204      204           
  Branches       40       40           
=======================================
  Hits          176      176           
  Misses         26       26           
  Partials        2        2           
Flag Coverage Δ
dashboards-search-relevance 86.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...search_components/search_configs/search_config.tsx 50.00% <ø> (ø)

@evankielley
Copy link
Contributor Author

@evankielley Thank you for your contribution to the DSR plugin! Could please add a screenshot of the page before and after the changes. Also, the identical error message appears in multiple files; can we make it a constant? Please disregard the yarn.lock file change. The maintainers are addressing it now.

No problem, will do!

@evankielley
Copy link
Contributor Author

Ready for re-review

Copy link
Collaborator

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Can you rebase it since the yarn.lock fix should resolved #271

@evankielley
Copy link
Contributor Author

The change looks good to me. Can you rebase it since the yarn.lock fix should resolved #271

Oh nice. Will do. Sorry I forgot that earlier.

Signed-off-by: Evan Kielley <[email protected]>
@evankielley
Copy link
Contributor Author

Ready

@noCharger
Copy link
Collaborator

https://github.com/opensearch-project/dashboards-search-relevance/actions/runs/6090046855/job/16524884733?pr=270

| Exception in thread "main" java.lang.IllegalStateException: failed to load plugin opensearch-amazon-kendra-intelligent-ranking-3.0.0.0 due to jar hell
| 	at org.opensearch.plugins.PluginsService.checkBundleJarHell(PluginsService.java:681)
| 	at org.opensearch.plugins.InstallPluginCommand.jarHellCheck(InstallPluginCommand.java:862)
| 	at org.opensearch.plugins.InstallPluginCommand.loadPluginInfo(InstallPluginCommand.java:830)
| 	at org.opensearch.plugins.InstallPluginCommand.installPlugin(InstallPluginCommand.java:875)
| 	at org.opensearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:276)
| 	at org.opensearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:250)
| 	at org.opensearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:104)
| 	at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
| 	at org.opensearch.cli.MultiCommand.execute(MultiCommand.java:104)
| 	at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
| 	at org.opensearch.cli.Command.main(Command.java:101)
| 	at org.opensearch.plugins.PluginCli.main(PluginCli.java:[66](https://github.com/opensearch-project/dashboards-search-relevance/actions/runs/6090046855/job/16524884733?pr=270#step:7:67))
| Caused by: java.lang.IllegalStateException: jar hell!
| class: org.apache.commons.logging.impl.AvalonLogger
| jar1: /home/runner/work/dashboards-search-relevance/dashboards-search-relevance/search-processor/amazon-kendra-intelligent-ranking/build/testclusters/integTest-0/distro/3.0.0-ARCHIVE/plugins/.installing-1110415763305379888/commons-logging-1.2.jar
| jar2: /home/runner/work/dashboards-search-relevance/dashboards-search-relevance/search-processor/amazon-kendra-intelligent-ranking/build/testclusters/integTest-0/distro/3.0.0-ARCHIVE/lib/commons-logging-1.2.jar
| 	at org.opensearch.bootstrap.JarHell.checkClass(JarHell.java:316)
| 	at org.opensearch.bootstrap.JarHell.checkJarHell(JarHell.java:215)
| 	at org.opensearch.plugins.PluginsService.checkBundleJarHell(PluginsService.java:[67](https://github.com/opensearch-project/dashboards-search-relevance/actions/runs/6090046855/job/16524884733?pr=270#step:7:68)9)
| 	... 11 more

FAILURE: Build failed with an exception.

The jar hell issue should be resolved after the new artifacts for OpenSearch core are available. Merge this PR now.

@noCharger noCharger merged commit dff6559 into opensearch-project:main Sep 5, 2023
10 of 12 checks passed
github-actions bot added a commit that referenced this pull request Sep 5, 2023
* Changing select index error message to be more informative (#173)

Signed-off-by: Evan Kielley <[email protected]>

* Changing select index error message to be a constant to keep code DRY (#173)

Signed-off-by: Evan Kielley <[email protected]>

* Reverting change to yarn.lock

Signed-off-by: Evan Kielley <[email protected]>

---------

Signed-off-by: Evan Kielley <[email protected]>
(cherry picked from commit dff6559)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
noCharger pushed a commit that referenced this pull request Sep 5, 2023
…) (#275)

* Changing select index error message to be more informative (#173)



* Changing select index error message to be a constant to keep code DRY (#173)



* Reverting change to yarn.lock



---------


(cherry picked from commit dff6559)

Signed-off-by: Evan Kielley <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Good first issue] Update message shown when a single index is selected in search comparison tool
2 participants