Skip to content

Conversation

@qfritz
Copy link

@qfritz qfritz commented Jan 21, 2025

_ignore_default_error will ignore errors like:

"Default value must match first schema in union with type: null"

...which I agree are errors but are painful to fix when backporting stuff.

WDYT?

@qfritz
Copy link
Author

qfritz commented Jan 23, 2025

Ah, deploy preview CI step is broken 🤔

@marcosschroh
Copy link
Owner

_ignore_default_error will ignore errors like:

"Default value must match first schema in union with type: null"

...which I agree are errors but are painful to fix when backporting stuff.

WDYT?

Thanks for the PR. I can be a good idea but we need to fix this in a different way. In the AvroSchema.init method you can get the extra fastavro property _ignore_default_error and use it when creating the schema: avro_schema = schema.AvroSchema(deployment_schema). The default value should be False to avoid a breaking change.

The deploy-preview step fails due the github actions because it still does not support deployments from a fork

Copy link
Owner

@marcosschroh marcosschroh left a comment

Choose a reason for hiding this comment

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

Use the property in the AvroSchema.init _ignore_default_error and use it when creating the schema.

@qfritz qfritz force-pushed the patch-1 branch 2 times, most recently from c44ae11 to 4d19c3f Compare January 27, 2025 08:15
@qfritz
Copy link
Author

qfritz commented Jan 27, 2025

Hi again, thanks for reviewing this PR.
I tried to apply your suggestion and amended my commit (sorry, my python is rusty, i hope I got it right).

@qfritz qfritz force-pushed the patch-1 branch 2 times, most recently from 087a8a3 to bcd4e4a Compare January 27, 2025 08:18
@qfritz qfritz changed the title feat(schema): set _ignore_default_error to True feat(schema): allow set _ignore_default_error to True Jan 27, 2025
@qfritz
Copy link
Author

qfritz commented Jan 27, 2025

Added tests! 🎉
I made sure that they pass. I'm not sure about all the init changes but if i didn't update both BaseSchema and AvroSchema, tests were failing.

@qfritz qfritz requested a review from marcosschroh January 27, 2025 18:12
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