Skip to content

creating new circular-json safe stringify and replacing one call#6772

Merged
betodealmeida merged 1 commit intoapache:masterfrom
lyft:VIZ-163
Jan 29, 2019
Merged

creating new circular-json safe stringify and replacing one call#6772
betodealmeida merged 1 commit intoapache:masterfrom
lyft:VIZ-163

Conversation

@mrmcduff
Copy link
Contributor

Fixes one instance of #6761

Why not replace every call to JSON.stringify in this PR?

  • If there is an unknown bug in the source, this limits the exposure (and no other crashes have yet been reported)
  • A follow-up PR should eliminate all unprotected references to JSON.stringify

Creating a time-series table using sample data without the PR:
screen shot 2019-01-25 at 3 51 01 pm

Creating a time-series table using sample data with this PR:
screen shot 2019-01-28 at 11 17 31 am

@codecov-io
Copy link

Codecov Report

Merging #6772 into master will decrease coverage by 17.33%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #6772       +/-   ##
===========================================
- Coverage   73.42%   56.09%   -17.34%     
===========================================
  Files          75      526      +451     
  Lines       10084    23230    +13146     
  Branches        0     2779     +2779     
===========================================
+ Hits         7404    13030     +5626     
- Misses       2680     9791     +7111     
- Partials        0      409      +409
Impacted Files Coverage Δ
superset/assets/src/utils/safeStringify.ts 100% <100%> (ø)
superset/assets/src/explore/exploreUtils.js 80.85% <100%> (ø)
superset/assets/src/components/Checkbox.jsx 100% <0%> (ø)
...ations/deckgl/layers/Polygon/PolygonChartPlugin.js 0% <0%> (ø)
...ets/src/dashboard/components/dnd/DragDroppable.jsx 97.14% <0%> (ø)
...c/visualizations/deckgl/layers/Polygon/Polygon.jsx 0% <0%> (ø)
...ssets/src/visualizations/presets/MapChartPreset.js 0% <0%> (ø)
superset/assets/src/components/EditableTitle.jsx 77.14% <0%> (ø)
...assets/src/visualizations/Iframe/transformProps.js 0% <0%> (ø)
superset/assets/src/setup/setupPlugins.js 0% <0%> (ø)
... and 443 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83ee917...2635bf0. Read the comment docs.

@betodealmeida
Copy link
Member

This looks great! Thanks for the unit tests, and for adding the ASF headers. I'm fine with changing other calls in a later PR.

@mistercrunch
Copy link
Member

LGTM

@betodealmeida betodealmeida merged commit 11a7ad0 into apache:master Jan 29, 2019
john-bodley added a commit that referenced this pull request Jan 30, 2019
john-bodley added a commit that referenced this pull request Jan 31, 2019
* Revert "creating new circular-json safe stringify and replacing one call (#6772)"

This reverts commit 11a7ad0.

* Revert "Improve Unicode support for MSSQL (#6690)"

This reverts commit c44ae61.

* Revert "Fix uniqueness constraints on tables table (#6718)"

This reverts commit c4fb7a0.
xtinec pushed a commit that referenced this pull request Feb 5, 2019
@kristw kristw mentioned this pull request Feb 7, 2019
3 tasks
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 First shipped in 0.34.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels v0.31 🚢 0.34.0 First shipped in 0.34.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants