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

Change: correctly propagate ElasticSearch errors to the UI #1402

Merged
merged 2 commits into from Nov 20, 2016
Merged

Change: correctly propagate ElasticSearch errors to the UI #1402

merged 2 commits into from Nov 20, 2016

Conversation

adamlwgriffiths
Copy link
Contributor

@adamlwgriffiths adamlwgriffiths commented Nov 17, 2016

All erronous ES queries have been displaying an error stating the JSON could not be decoded, regardless of the actual error (HTTP, syntax, auth, etc).

This modification ensures any request error is propagated back.
It seems that the main issue was requests errors being non-json-serializable due to the usage of both " and ' characters.
The abstraction of the _get_mapping function also made it difficult to get the root error instead of just the exception, this has been modified to be similar to run_query and now returns a tuple of (mappings, error).

@arikfr
Copy link
Member

arikfr commented Nov 17, 2016

Thank you for doing some house keeping there as well! The ElasticSearch data source definitely needs some more love.

@adamlwgriffiths
Copy link
Contributor Author

Just noticed that I've mixed logging and logger usage (always do that =). I'll fix this tomorrow and update the PR.

On 17 Nov 2016, at 7:23 pm, Arik Fraimovich notifications@github.com wrote:

Thank you for doing some house keeping there as well! The ElasticSearch data source definitely needs some more love.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Handle and correctly print various errors which
we're being obscured as "Unable to serialise JSON"
errors.
Requests' exceptions also have non-json serializable
characters, and so have to be converted to a safer format.
@adamlwgriffiths
Copy link
Contributor Author

Ok, should be good to go.
Changed incorrect usage of logging to logger.
Moved some of the syntax changes from the error commit back to the PEP-8 commit.

@arikfr arikfr changed the title Correctly propagate ElasticSearch errors to the UI Change: correctly propagate ElasticSearch errors to the UI Nov 20, 2016
@arikfr arikfr merged commit e65ec8c into getredash:master Nov 20, 2016
arikfr added a commit that referenced this pull request Nov 25, 2016
dairyo pushed a commit to KiiCorp/redash that referenced this pull request Mar 1, 2019
Change: correctly propagate ElasticSearch errors to the UI
dairyo pushed a commit to KiiCorp/redash that referenced this pull request Mar 1, 2019
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.

2 participants