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

AVRO-2539: fix nullable types for avro to thrift #2396

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nryanov
Copy link

@nryanov nryanov commented Jul 27, 2023

What is the purpose of the change

Main reason of this PR: fix issue with backward compatibility of converted thrift-classes into avro-classes.
Minor fix: pom.xml in thrift submodule (maven-antrun-plugin)

Previous PR: #639

Verifying this change

This change added tests and can be verified as follows:

  • Added test that validates that structs of different versions are compatible
  • Added test that validates that optional thrift fields converted into avro with correct union type

Documentation

  • Does this pull request introduce a new feature? (no)

@github-actions github-actions bot added Java Pull Requests for Java binding build labels Jul 27, 2023
}

@org.junit.jupiter.api.Test
public void testBackwardCompatibility() {
Copy link
Author

Choose a reason for hiding this comment

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

Additional test that verify that actual fix works in terms of compatibility

@nryanov
Copy link
Author

nryanov commented Jul 27, 2023

@clesaec
If you will have a time, please, review this PR (this is a copy of #639)

Copy link
Contributor

@clesaec clesaec left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Java Pull Requests for Java binding
Projects
None yet
2 participants