[sqllab] remove limiting at the display level #5413
Conversation
timifasubaa
changed the title
| return json_success( | ||
| json.dumps( | ||
| payload_json, default=utils.json_iso_dttm_ser, ignore_nan=True)) | ||
| return json_success(utils.zlib_decompress_to_string(blob)) |
There was a problem hiding this comment.
Why are the default and ignore_nan arguments no longer valid?
There was a problem hiding this comment.
When the blob is written in celery, it is already set with those parameters.
https://github.com/apache/incubator-superset/blob/master/superset/sql_lab.py#L229
There was a problem hiding this comment.
I added ignore-nan
There was a problem hiding this comment.
Turns out ignore_nan is breaking this method. I believe it's a simplejson arg only
timifasubaa
force-pushed
the
remove_sqllab_frontend_limits
branch
5 times, most recently
from
ceccdfd to
8489ae5
Compare
timifasubaa
force-pushed
the
remove_sqllab_frontend_limits
branch
from
8489ae5 to
ea9b12a
Compare
Codecov Report
@@ Coverage Diff @@
## master #5413 +/- ##
=========================================
+ Coverage 59.09% 59.1% +0.01%
=========================================
Files 372 372
Lines 23747 23742 -5
Branches 2758 2758
=========================================
Hits 14033 14033
+ Misses 9699 9694 -5
Partials 15 15
Continue to review full report at Codecov.
|
|
I missed this a while back, but how do we allow for users to do largish CSV extracts when in async mode? @timifasubaa @john-bodley |
mistercrunch
added
🏷️ bot
Especially as we now enforce the limit into the query(#5295), there is no longer a need to reduce the payload before sending to the frontend. This PR removes that logic.
This should also improve performance as we no longer need to json.loads and dumps the data in the middle.
@john-bodley @mistercrunch