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 optional fields in json #216

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

Conversation

phshah95
Copy link

Issue #, if available:

Description of changes:

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

@phshah95 phshah95 force-pushed the json_fix_optional branch 2 times, most recently from 086ac3a to 3004db8 Compare September 25, 2022 22:49
@phshah95
Copy link
Author

@mohitpali any chance you can leave some feedback on this small change? It is preventing us from using schemas like the one defined in the test. Thank you

@phshah95
Copy link
Author

also not sure why that schema is being parsed as anyOf instead of oneOf..maybe the fix is there instead. Although, it does seem logical to included anyOf in this check too.

@YangSan0622
Copy link
Contributor

YangSan0622 commented Sep 26, 2022

Hello, do you mind create a issue to describe the problem we are trying to resolve here so we can have more context before we start the review process?

@phshah95
Copy link
Author

phshah95 commented Sep 27, 2022

made an issue here @YangSan0622

@mohitpali
Copy link
Contributor

Please add unit tests or fix them to help us understand the issue better - tests are failing with error -

 [INFO] Running com.amazonaws.services.schemaregistry.kafkaconnect.jsonschema.ToConnectTest
Error:  Tests run: 62, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.098 s <<< FAILURE! - in com.amazonaws.services.schemaregistry.kafkaconnect.jsonschema.ToConnectTest
Error:  testSchemaCache_size_toConnectConversion  Time elapsed: 0.011 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <1> but was: <0>

@YangSan0622
Copy link
Contributor

It looks like the test failure mentioned is still there. Do we know what is going on?

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.

3 participants