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

Improve error message if setting is not found #17230

Merged
merged 1 commit into from Mar 22, 2016

Conversation

Projects
None yet
7 participants
@s1monw
Contributor

s1monw commented Mar 21, 2016

We can do better than just throwing an error when we don't find a
setting. It's actually trivial to leverage lucenes slow LD StringDistance
to find possible candiates for a setting to detect missspellings and suggest
a possible setting.
This commit adds error messages like:

  • unknown setting [index.numbe_of_replica] did you mean [index.number_of_replicas]?

rather than just reporting the setting as unknown

Improve error message if setting is not found
We can do better than just throwing an error when we don't find a
setting. It's actually trivial to leverage lucenes slow LD StringDistance
to find possible candiates for a setting to detect missspellings and suggest
a possible setting.
This commit adds error messages like:

 * `unknown setting [index.numbe_of_replica] did you mean [index.number_of_replicas]?`

rather than just reporting the setting as unknown
@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Mar 21, 2016

Contributor

thanks @rmuir for putting this on the table - it's an awesome idea :)

Contributor

s1monw commented Mar 21, 2016

thanks @rmuir for putting this on the table - it's an awesome idea :)

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Mar 21, 2016

Contributor

+1, great improvement! just like the JDK.

Note: if later we want better support for transpositions, there is also LuceneLevenshteinDistance and JaroWinkler which are geared at those too. Someone can always explore those, this one should do well though (esp with 0.7)

Contributor

rmuir commented Mar 21, 2016

+1, great improvement! just like the JDK.

Note: if later we want better support for transpositions, there is also LuceneLevenshteinDistance and JaroWinkler which are geared at those too. Someone can always explore those, this one should do well though (esp with 0.7)

@mikemccand

This comment has been minimized.

Show comment
Hide comment
@mikemccand

mikemccand Mar 21, 2016

Contributor

+1, very cool!

Contributor

mikemccand commented Mar 21, 2016

+1, very cool!

@kimchy

This comment has been minimized.

Show comment
Hide comment
@kimchy

kimchy Mar 21, 2016

Member

I wonder what is the levenshtein distance between a @rmuir and great features is :)

Member

kimchy commented Mar 21, 2016

I wonder what is the levenshtein distance between a @rmuir and great features is :)

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Mar 21, 2016

Contributor

LGTM

Contributor

nik9000 commented Mar 21, 2016

LGTM

String msg = "unknown setting [" + key + "]";
List<String> keys = scoredKeys.stream().map((a) -> a.v2()).collect(Collectors.toList());
if (keys.isEmpty() == false) {
msg += " did you mean " + (keys.size() == 1 ? "[" + keys.get(0) + "]": "any of " + keys.toString()) + "?";

This comment has been minimized.

@dadoonet

dadoonet Mar 22, 2016

Member

I love this! This is so great for our users.

@dadoonet

dadoonet Mar 22, 2016

Member

I love this! This is so great for our users.

@bleskes

This comment has been minimized.

Show comment
Hide comment
@bleskes

bleskes Mar 22, 2016

Member

everything that prevents from staring at screen only to feel dumb later is great :)

Member

bleskes commented Mar 22, 2016

everything that prevents from staring at screen only to feel dumb later is great :)

@s1monw s1monw merged commit a0c68c2 into elastic:master Mar 22, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment