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 issue with Avro record default values in Kafka Connect converter #308

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jurgispods
Copy link

Issue #, if available: #307

Description of changes: See issue for description of the problem. This PR adds a check for JsonProperties.NULL_VALUE in addition to null to fix this problem.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jurgispods
Copy link
Author

Did anyone have a chance to look at this?

@YangSan0622
Copy link
Contributor

Can you add unit test for this feature and ensure the existing integration test is passing after this change?

@jurgispods
Copy link
Author

@YangSan0622 I copied the same unit test used in confluentinc/schema-registry#1928 - mvn test runs successfully.

@YangSan0622
Copy link
Contributor

Have you run the integration tests in https://github.com/awslabs/aws-glue-schema-registry/tree/master/integration-tests?

@jurgispods
Copy link
Author

@YangSan0622 Running mvn test from the integration-test directory runs through, but run-local-tests.sh fails with a few "Access denied" errors. Is there any documentation on how to run the tests? It seems the tests need an AWS account with certain permissions. If that is the case, it would be very helpful if you could run the tests in your environment.

@jurgispods
Copy link
Author

Hi, any update on this? I will gladly run the integration tests, given I know how to do so and I have permission to access the corresponding AWS resources. Otherwise, I am dependent on the help of a maintainer.

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.

2 participants