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

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

Merged
merged 1 commit into from
Jan 29, 2019

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 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants