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

Parameterized exception messages #11981

Merged
merged 1 commit into from Jul 1, 2015
Merged

Parameterized exception messages #11981

merged 1 commit into from Jul 1, 2015

Conversation

uboness
Copy link
Contributor

@uboness uboness commented Jul 1, 2015

Added dynamic arguments to ElasticsearchException, ElasticsearchParseException and ElasticsearchTimeoutException.

This helps keeping the exception messages clean and readable and promotes consistency around wrapping dynamic args with [ and ].

This is just the start, we need to propagate this to all exceptions deriving from ElasticsearchException. Also, work started on standardizing on lower case logging & exception messages. We need to be consistent here...

  • Uses the same LoggerMessageFormat as used by our logging infrastructure.

@s1monw
Copy link
Contributor

s1monw commented Jul 1, 2015

LGTM thanks for doing this

@nik9000
Copy link
Member

nik9000 commented Jul 1, 2015

Neat idea reusing the logging message formatter for exceptions. Nice for consistency. I see a lot of [{}] in there. Maybe make something convenient for that? Something along the lines of [] being interpreted like [{}].

Added dynamic arguments to `ElasticsearchException`, `ElasticsearchParseException` and `ElasticsearchTimeoutException`.

This helps keeping the exception messages clean and readable and promotes consistency around wrapping dynamic args with `[` and `]`.

This is just the start, we need to propagate this to all exceptions deriving from `ElasticsearchException`. Also, work started on standardizing on lower case logging & exception messages. We need to be consistent here...

 - Uses the same `LoggerMessageFormat` as used by our logging infrastructure.
@uboness
Copy link
Contributor Author

uboness commented Jul 1, 2015

I see a lot of [{}] in there. Maybe make something convenient for that? Something along the lines of [] being interpreted like [{}].

now that's a neat idea as well :) I'll get this one in first and take it from there

@nik9000
Copy link
Member

nik9000 commented Jul 1, 2015

now that's a neat idea as well :) I'll get this one in first and take it from there

Cool!

@uboness uboness merged commit 6021bd8 into elastic:master Jul 1, 2015
@kevinkluge kevinkluge removed the review label Jul 1, 2015
@lcawl lcawl added :Core/Infra/Core Core issues without another label and removed :Exceptions labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >enhancement v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants