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

Add ability to download schema references #156

Merged
merged 2 commits into from Jan 8, 2024
Merged

Conversation

ferozed
Copy link
Contributor

@ferozed ferozed commented Jan 1, 2024

Added ability to download schema references.

A new configuration downloadReferences: Boolean was added to DownloadSchema.

When set to true, and the schema has references, the referenced schemas are also downloaded. This will allow POJO generation to succeed from schema with references.

@ImFlog
Copy link
Owner

ImFlog commented Jan 3, 2024

@ferozed the integration tests are failing. There is a special gradle task for them that you should be able to run locally (if you have Docker installed).

Also, after looking a bit in the confluent code I wonder if there are not an integrated way to do this directly when parsing the schema using `toNormalize before retrieving the schema ?

@ferozed
Copy link
Contributor Author

ferozed commented Jan 3, 2024

Also, after looking a bit in the confluent code I wonder if there are not an integrated way to do this directly when parsing the schema using `toNormalize before retrieving the schema ?

Right... but this will only work for AVRO schemas?

@ferozed
Copy link
Contributor Author

ferozed commented Jan 3, 2024

Yeah... so the integration tests are failing because the new config I added is a required value. Is there any way to make it optional? I tried using @Optional annotation but that didnt work.

@ferozed
Copy link
Contributor Author

ferozed commented Jan 4, 2024

Ok... i kinda worked around it by putting the downloadReferences configuration per subject, instead of it being global.

@ImFlog
Copy link
Owner

ImFlog commented Jan 5, 2024

Starts to look good !
Only issue is that you have no way to set the flag to true as no property in the extension is mapped to the subject.
You should probably add an integration test with the flag set to true and verify that 2 files are created.

Also we should update the readme with the new parameter.

@ImFlog
Copy link
Owner

ImFlog commented Jan 8, 2024

LGTM !
I think I will try to release this with #160.

I keep you posted when I found the time to do so.

Thank you for the contribution 🙏

@ImFlog ImFlog merged commit 9929290 into ImFlog:master Jan 8, 2024
1 check passed
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.

None yet

2 participants