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

Use Jackson (instead of Moshi) as the JSON parser library #464

Closed
lxhoan opened this issue Jul 5, 2018 · 6 comments
Closed

Use Jackson (instead of Moshi) as the JSON parser library #464

lxhoan opened this issue Jul 5, 2018 · 6 comments

Comments

@lxhoan
Copy link
Contributor

lxhoan commented Jul 5, 2018

Currently JSON parser in influxdb-java is Moshi. However we are going to support MessgagePack format (issue #389).
The Retrofit converter (referred to in Retrofit wiki page) for MessagePack is https://github.com/komamitsu/retrofit-converter-msgpack

This convertor depends on Jackson. To ensure the consistency we should switch to Jackson as well
(Moreover, seems we can also consider to drop JSON in influxdb-java due to some limitations of JSON, refer to this influxdb Proposal: Stop using JSON as the standard response format for more information)

@lxhoan lxhoan added this to the 2.12 milestone Jul 5, 2018
@majst01
Copy link
Collaborator

majst01 commented Jul 5, 2018

Hi Hoan,

we should put this into the 3.x milestone, because dropping json support means dropping support for influxdb versions < 1.4 which would be a major change from the user perspective.

@lxhoan
Copy link
Contributor Author

lxhoan commented Jul 5, 2018

Hi Stefan,

(Moreover, seems we can also consider to drop JSON in influxdb-java due to some limitations of JSON, refer to this influxdb Proposal: Stop using JSON as the standard response format for more information)

This sentence is a proposal and not in the scope of this issue.

Basically this issue is to switch to Jackson only (for MessagePack and JSON), we are not going to drop JSON in 2.12

@majst01
Copy link
Collaborator

majst01 commented Jul 5, 2018

On question, why do we need a json deserializer at all if we switch to messagepack ?
https://github.com/msgpack/msgpack-java for example does not have any external dependencies, which for me would be a perfect fit for a core library usage as in our case.

@majst01
Copy link
Collaborator

majst01 commented Jul 5, 2018

Who decided that ?

@lxhoan
Copy link
Contributor Author

lxhoan commented Jul 5, 2018

https://github.com/msgpack/msgpack-java for example does not have any external dependencies, which for me would be a perfect fit for a core library usage as in our case.

This msgpack-core does not have object binding, and this library also suggests we had best use jackson-databind (which depends on jackson-core) for object binding (you can check this https://github.com/msgpack/msgpack-java/blob/develop/msgpack-jackson/README.md)

@lxhoan
Copy link
Contributor Author

lxhoan commented Jul 11, 2018

closed since I will implement a MessagePack retrofit convertor for influxdb-java

@lxhoan lxhoan closed this as completed Jul 11, 2018
@lxhoan lxhoan removed this from the 2.12 milestone Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants