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

Fix Avro non-record type deserialization #324

Merged
merged 2 commits into from
Feb 13, 2025
Merged

Conversation

rohitsanj
Copy link
Contributor

@rohitsanj rohitsanj commented Feb 12, 2025

Summary of Changes

Fixes #220

Proof that it works:

Screenshot 2025-02-12 at 10 46 21 AM

Why am I not writing an integration test for this? Well, that would require us to fix this exact problem on the serialization side of things, and that can wait for now.

Any additional details or context that should be provided?

Pull request checklist

Please check if your PR fulfills the following (if applicable):

  • Tests:
    • Added new
    • Updated existing
    • Deleted existing
  • Have you validated this change locally against a running instance of the Quarkus dev server?
    make quarkus-dev
  • Have you validated this change against a locally running native executable?
    make mvn-package-native && ./target/ide-sidecar-*-runner

@Copilot Copilot bot review requested due to automatic review settings February 12, 2025 18:47
@rohitsanj rohitsanj requested a review from a team as a code owner February 12, 2025 18:47

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/main/java/io/confluent/idesidecar/restapi/messageviewer/RecordDeserializer.java:146

  • Ensure that there are tests verifying the handling of non-record Avro types.
return OBJECT_MAPPER.valueToTree(genericObject);
@Cerchie
Copy link
Contributor

Cerchie commented Feb 12, 2025

nice! how do I set up this end-to-end clicktesting like in the screenshot?

@rohitsanj
Copy link
Contributor Author

nice! how do I set up this end-to-end clicktesting like in the screenshot?

Here are the steps:

  1. Build the native executable against this branch using make clean mvn-package-native-no-tests
  2. Copy the built executable into the bin directory in your vscode repository clone, cp ./target/ide-sidecar-0.153-runner <wherever-you've-cloned-vscode>/bin
  3. Set the .versions/ide-sidecar.txt version to 0.153.0
  4. Open vscode repo in vscode and hit F5 to start in debug mode
  5. Login to our DTX Confluent Cloud org, expand custom-data-env, click on custom-data-cluster, and consume WeatherData topic. You should see the key column deserialized as timestamps correctly.

@sonarqube-confluent
Copy link

Passed

Analysis Details

0 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage 66.70% Coverage (79.70% Estimated after merge)
  • Duplications No duplication information (0.60% Estimated after merge)

Project ID: ide-sidecar

View in SonarQube

@jlrobins
Copy link
Contributor

jlrobins commented Feb 13, 2025

I have verified that this build works for the WeatherData topic, whose key schema specifies an AVRO primitive long, and whose body schema is an AVRO record.

Can consume all ~874,459+ records from the topic. Set consumption mode to 'latest' to then get a gentle ~1 record per minute of realtime weather samples.

@rohitsanj rohitsanj merged commit 4606cda into main Feb 13, 2025
2 checks passed
@rohitsanj rohitsanj deleted the fix-avro-deser-non-record branch February 13, 2025 17:57
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

Successfully merging this pull request may close these issues.

Sidecar message consumption AVRO deserialization assumes records
3 participants