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

possible move to Jackson from Gson for kafka #655

Closed
a1shadows opened this issue Mar 31, 2024 · 4 comments
Closed

possible move to Jackson from Gson for kafka #655

a1shadows opened this issue Mar 31, 2024 · 4 comments
Assignees

Comments

@a1shadows
Copy link
Collaborator

based on discussion with @baulea and @authorjapps , we need to investigate why Gson was used and if we should move fully to Jackson.

discussed in #648

@baulea
Copy link
Contributor

baulea commented Mar 31, 2024

Hi @a1shadows ,
I could have a look at that next week.

@baulea
Copy link
Contributor

baulea commented Apr 9, 2024

Hi @a1shadows
GSON internally uses a ToNumberPolicy.DOUBLE and converts JSON numbers into Double.

E.g. in kafka-testing/src/test/resources/kafka/produce/test_kafka_produce_int_key.json, the request contains an "Integer" key "101" in the produce operation, but during the unload operation the assertion checks for a "Double" key.

{
    "scenarioName": "Produce a message to kafka topic - int Key",
    "steps": [
        {...
            "operation": "produce",
            "request": {
                "recordType" : "RAW",
                "records": [
                    {
                        **"key": 101,**
                        "value": "Hello World"
                    }
                ]
            }, ...
        },
        {  ...
            "operation": "unload",
     ...
            "assertions": {
                "size": 1,
                "records": [
                    {
                        **"key": 101.0,**
                        "value": "Hello World"
                    }
                ]
            }
        }
    ]
}

Moving to Jackson, I did not find a suitable build-in replacement for this mechanism. So we would have recreate this behaviour programmatically to prevent the tests from failing.

I don't know whether this implicit conversion Integer->Double just sneaked in because GSON had been used or if it is an important feature.

Could we change "key" in the produce section of the unit tests to double? This would break the current functionality, but it would be more consistent in my opinion.

@a1shadows
Copy link
Collaborator Author

Hey, I'll take a look at this and get back to you

@authorjapps
Copy link
Owner

Hey Guys @baulea, @a1shadows, Thanks for looking at it. But imo gson is still fine as long as it doesn't do any harm, also it's great where it has solved a usecase easily.
I don't think we will gain a lot by replacing this with jackson as currently this only does the serde tasks around 1% of the scenarios(e.g. kafka headers etc).

So my advice would be to keep this as a low proprity TECHDEBT or withdraw this ticket until it blocks anyone or any problem scenario.
It's worth checking other functional issues which have ben raised recently as well the old ones which might benefit the framework's overall capability.

Thanks for understanding...

@a1shadows a1shadows closed this as not planned Won't fix, can't repro, duplicate, stale May 19, 2024
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

3 participants