Render infinite values to valid JSON values. Fixes #813 #1710

Merged
merged 1 commit into from Nov 29, 2016

Projects

None yet

5 participants

@rehevkor5
Contributor
rehevkor5 commented Sep 30, 2016 edited
  • The 'json' and 'jsonp' formats output float('inf') as 1e9999, float
    ('-inf') as -1e9999, and float('nan') as null
  • The 'dygraph' format (a JSON-based format) outputs the same as
    Infinity, -Infinity, and null (see http://dygraphs.com/tests/gviz-infinity.html)
  • Minor updates to documents to make sure commands can be run without errors or problems
@rehevkor5
Contributor

Disclaimer: I am not really a Python developer, so hopefully I didn't do anything terribly thickheaded.

@rehevkor5 rehevkor5 Render infinite values to valid JSON values. Fixes #813
- The 'json' and 'jsonp' formats output float('inf') as 1e9999, float
('-inf') as -1e9999, and float('nan') as null
- The 'dygraph' format (a JSON-based format) outputs the same as
Infinity, -Infinity, and null (see http://dygraphs
.com/tests/gviz-infinity.html)
- Minor updates to documents to make sure commands can be run without errors or problems
a971bcd
@codecov-io
codecov-io commented Sep 30, 2016 edited

Current coverage is 65.65% (diff: 73.17%)

Merging #1710 into master will increase coverage by 0.03%

@@             master      #1710   diff @@
==========================================
  Files            54         55     +1   
  Lines          6122       6161    +39   
  Methods           0          0          
  Messages          0          0          
  Branches       1210       1222    +12   
==========================================
+ Hits           4017       4045    +28   
- Misses         1892       1899     +7   
- Partials        213        217     +4   

Powered by Codecov. Last update 6811d20...a971bcd

@deniszh
Member
deniszh commented Oct 1, 2016

LGTM.
I would prefer separate commits for docs and functionality itself, but maybe I'm nitpicking here.
Any other reviewers? /cc @graphite-project/committers ?

@obfuscurity
Member

@torkelo Would this cause any problems for Grafana?

@torkelo
Contributor
torkelo commented Nov 27, 2016

No, I think this is an improvement, if I recall correctly the Infinite value returned by graphite today cause invalid json errors

@obfuscurity
Member

Note that I want to merge this in but not until #1690 is resolved. There is a related branch that's already pretty complicated that I'd like to see go in first, then we can rebase this one.

@rehevkor5
Contributor

@obfuscurity regarding your question about whether this might cause problems for Grafana: I created this PR in order to resolve problems with Grafana, actually! I have seen graphs refuse to render if Infinity is in the JSON returned by Graphite.

@obfuscurity
Member

Going to merge this now. Discussed with @DanCech, he said he'd rebase this into his ongoing work related to #1690.

@obfuscurity obfuscurity merged commit db726ab into graphite-project:master Nov 29, 2016

3 checks passed

codecov/patch 73.17% of diff hit (target 65.61%)
Details
codecov/project 65.65% (+0.03%) compared to 6811d20
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rehevkor5 rehevkor5 deleted the rehevkor5:issue-813 branch Nov 30, 2016
@rehevkor5 rehevkor5 added a commit to rehevkor5/graphite-web that referenced this pull request Jan 3, 2017
@rehevkor5 rehevkor5 Fix JSON performance regression caused by #1710
- Removed slow FloatEncoder, using json.dumps again instead
- Still getting good behavior with regard to inf, -inf, and NaN
by converting the values in the data to max & min float value and
None respectively. allow_nan=False prevents bad values from
accidentally being rendered in the JSON.
40e6263
@rehevkor5 rehevkor5 added a commit to rehevkor5/graphite-web that referenced this pull request Jan 3, 2017
@rehevkor5 rehevkor5 Fix JSON performance regression caused by #1710
- Removed slow FloatEncoder, using json.dumps again instead
- Still getting good behavior with regard to inf, -inf, and NaN
by converting the values in the data to max & min float value and
None respectively. allow_nan=False prevents bad values from
accidentally being rendered in the JSON.
0546308
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment