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.

Sign up for GitHub

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

Jump to bottom

Naming rules inconsistencies #414

Closed
papanikge opened this issue Jun 25, 2024 · 3 comments
Closed

Naming rules inconsistencies #414

papanikge opened this issue Jun 25, 2024 · 3 comments

Comments

@papanikge
Copy link
Contributor

Hello all,

Here's the situation. Apache Avro spec has some naming guidelines for Record, enums and fixed. This library correctly checks that and fails when a file has a badly named field.

However, the official Avro library does not 😢
avro-tools tojson <badly-named-file.ocf> happily reads the file (same goes for AWS - I think that's because they use Java)

There is a discussion about it here in the mailing list, and
furthermore Python seems to have chosen to allow for disabling this validation

Because of that last note, I think we should consider adding this optional functionality in this library too. What do you think? I can do a PR if you're on board.
@nrwiersma
Copy link
Member

Well, I will not pretend to like this. This is some serious ass backward stuff. Asking other libraries to cripple themselves cause other people cannot read a spec or make a decision is really not great.

That said, if it is turned off by default and is documented as counter spec, and does not add too much complexity, it would be accepted.

@papanikge papanikge mentioned this issue Jul 3, 2024
Merged
@papanikge
Copy link
Contributor Author

papanikge commented Jul 3, 2024 •
edited
Loading

Opened #415
Unfortunately I had to do property-drilling for that boolean configuration :/
Also I had to add the WithSkipNameValidation func in two spots, both in ocf and in schema file in the avro package.

Feel free to roast it and suggest any alternatives :) I'm not very accustomed to this repo's code yet.

I tested it in our systems using my fork and all our suite's tests seem to pass even for badly named files.
However, I would like to add some more tests here too.

@nrwiersma
Copy link
Member

Fixed by #415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nrwiersma @papanikge