imply-cheddar opened a new pull request, #14004:
URL: https://github.com/apache/druid/pull/14004
This is the 3rd take at creating a unified Exception in the DruidException.
Given that I had input on the previous two attempts, I figured I'd take the
next stab at offering something that matches the input that I had provided.
DruidException has class level javadoc discussing how it's all setup and
why, so I would recommend starting with that class-level javadoc and then
looking at some of the points where DruidExceptions are created (DruidPlanner
and various of the other places).
There is a compatibility layer in this PR that tries to continue mimicing
the old way that QueryException was getting serialized out, but the current PR
treats that as best-effort. Specifically, anything that started its life as a
DruidException will be an "unknown" error type when serialized back as a
QueryException. The error message should make it through, but the type
characterization will be different. This should only happen in the middle of
roll-forward/roll-back processes, so it will be short-lived and, given that it
preserves the message, is hopefully a decent compromise.
Additionally, as I was adjusting tests, I got tired of the SQL tests taking
roughly 5 minutes to run through the entire suite, so I looked into why it was
doing that and through moving some initialization around and fixing some
time-based async tests to be based on latches instead, was able to get that
timing down to ~1.5 minutes, so some of those changes are mixed in here as well.
#### Release note
A new unified exception for surfacing errors to users has been introduced.
It is partially compatible with the old way of reporting error messages,
response codes remain the same, all fields that previously existed on the
response will continue to exist and be populated, including the errorMessage.
But some error messages have changed to be more consumable by humans and some
cases will have the message restructured. Any clients that are actively
looking at the error message to make decisions on what to do should explore the
changes to the error messages that they care about, but if all a client looks
at is the response code, there should be no impact.
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency
checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md)
(Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
