Skip to content

Transform config error messages#8391

Open
mneedham wants to merge 2 commits intoapache:masterfrom
mneedham:transform-config-error-messages
Open

Transform config error messages#8391
mneedham wants to merge 2 commits intoapache:masterfrom
mneedham:transform-config-error-messages

Conversation

@mneedham
Copy link
Copy Markdown
Contributor

If you have a transform config that doesn't compile, the error message will only tell you that it's an invalid transform config, it doesn't say what the problem is.

This PR passes through the message from the underlying cause exception.

@mneedham mneedham force-pushed the transform-config-error-messages branch from f5ea923 to 359cee7 Compare March 23, 2022 16:52
Copy link
Copy Markdown
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

I don't follow why we need this change. The cause should be logged in the stack trace (unless the same exception repeated too many times and the stack trace is truncated in the log)

@mneedham
Copy link
Copy Markdown
Contributor Author

The full stack trace doesn't seem to get logged because the status code is 400. See:

https://github.com/apache/pinot/blob/master/pinot-controller/src/main/java/org/apache/pinot/controller/api/exception/ControllerApplicationException.java#L43

Also, I think it's a better user experience if you get a message back telling you what you've done wrong rather than a vague message that will result in someone coming to the OSS Slack and asking what it means, to then be told to look at the log file!

I should say this exact thing did happen on this thread here:
https://apache-pinot.slack.com/archives/C011C9JHN7R/p1648024250952359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants