redash
redash copied to clipboard
bugfix: fail to parse elasticsearch mappings
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
Elasticsearch query_runner may raise TypeError when dynamic setting presents.
e.g. for the following mapping:
{
"some_datasource": {
"mappings": {
"_meta": {...},
"date_detection":true,
"_source":Object{...},
"numeric_detection":false,
"dynamic_date_formats":[...],
"dynamic":"true",
"dynamic_templates":[...],
"properties":{...}
}
},
.....
}
date_detection and numeric_detection have bool values. BaseElasticSearch._get_query_mappings will raise "TypeError: argument of type 'bool' is not iterable" and fails to return any results for queries.
Check Elasticsearch docs for more details.
This PR add an additional condition check to ignore such fields.
@xiacongling , thanks for the PR! Do you think the error is relevant ? We've updated a lot of things now that we're Community-driven so. I'm rebase to master and run ci/cd tests
We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.
Codecov Report
Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
Project coverage is 60.73%. Comparing base (
113146e) to head (748740b). Report is 240 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #5123 +/- ##
==========================================
- Coverage 60.73% 60.73% -0.01%
==========================================
Files 153 153
Lines 12514 12516 +2
Branches 1695 1696 +1
==========================================
Hits 7601 7601
- Misses 4687 4689 +2
Partials 226 226
| Files | Coverage Δ | |
|---|---|---|
| redash/query_runner/elasticsearch.py | 15.51% <0.00%> (-0.11%) |
:arrow_down: |
Hi, @konnectr . It looks like the reason for the failure is a loss in test coverage. I'll take a look at it and try to add some test cases later this week. Thanks for the reply!