influxdb-java icon indicating copy to clipboard operation
influxdb-java copied to clipboard

fix bug: mapping from msgpack response to int, Integer, long, Long types works fine

Open chabapok opened this issue 5 years ago • 7 comments

All of the unit tests of the InfluxDBResultMapper is based on the assumption that all numbers in response from influxd is Double. It will be ok with JSON. But msgpack format is more precise.

So, InfluxDBResultMapper have errors. For example, it throws an exception when mapping number to int, Integer, long, Long types from msgpack responses, but works with json.

This PR fix it (with unit test).

chabapok avatar Apr 09 '20 22:04 chabapok

Nice catch, one small formatting nit:

[ERROR] /usr/src/mymaven/src/main/java/org/influxdb/impl/InfluxDBResultMapper.java:364:37: 'typecast' is not followed by whitespace. [WhitespaceAfter]
[ERROR] /usr/src/mymaven/src/main/java/org/influxdb/impl/InfluxDBResultMapper.java:385:33: 'typecast' is not followed by whitespace. [WhitespaceAfter]
[ERROR] /usr/src/mymaven/src/main/java/org/influxdb/impl/InfluxDBResultMapper.java:389:33: 'typecast' is not followed by whitespace. [WhitespaceAfter]

majst01 avatar Apr 10 '20 07:04 majst01

Codecov Report

Merging #671 into master will not change coverage by %. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #671   +/-   ##
=========================================
  Coverage     88.18%   88.18%           
  Complexity      722      722           
=========================================
  Files            69       69           
  Lines          2513     2513           
  Branches        268      268           
=========================================
  Hits           2216     2216           
  Misses          208      208           
  Partials         89       89           
Impacted Files Coverage Δ Complexity Δ
...n/java/org/influxdb/impl/InfluxDBResultMapper.java 88.81% <100.00%> (ø) 57.00 <0.00> (ø)

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 b3367fd...059089b. Read the comment docs.

codecov-io avatar Apr 10 '20 08:04 codecov-io

I'm reviewing this PR. Please, do not merge.

fmachado avatar Apr 10 '20 10:04 fmachado

Did anything ever happen with this one?

BrentonPoke avatar Dec 20 '20 06:12 BrentonPoke

What is a progress on this?

kaszperro avatar Apr 20 '21 10:04 kaszperro

Hi @fmachado, any update on this PR?

gbennardo avatar Mar 07 '23 17:03 gbennardo

I think he stopped working on this a long time ago. That's left a lot of PRs unmerged and most have likely gone to the influxdb-client-java repo.

BrentonPoke avatar Mar 12 '23 08:03 BrentonPoke