avrohugger
avrohugger copied to clipboard
julianpeeters
AVDL imports don't handle transitive deps / wrong file order
I've encountered a problem with transitive AVDL imports that don't work because of the file order.
This works:
A.avdl
B.avdl -> import idl "A.avdl";
C.avdl -> import idl "B.avdl";
This doesn't:
A.avdl -> import idl "B.avdl";
B.avdl -> import idl "C.avdl";
C.avdl
Error message is NoSuchElementException: key not found.
@mariussoutier Sorry to leave you hanging so long! I tried to reproduce the error with sbt-avrohugger 0.13.0, but both cases seem to succeed for me. (I tried with everything in the same namespaces, and then with everything in different namespaces and still everything worked, but I didn't check combos exhaustively); https://github.com/julianpeeters/avrohugger-transitive-dependencies-error
No problem, I also started to write a PR, but got distracted. I also wasn't sure how to access the imports.
Funny, when I check out your project and run sbt compile, I get exactly the error from this ticket:
[info] Compiling Avro IDL /Users/mariussoutier/Workspace/avrohugger-transitive-dependencies-error/src/main/avro/A.avdl
[trace] Stack trace suppressed: run last avro:generate for the full output.
[error] (avro:generate) java.util.NoSuchElementException: key not found: {"type":"record","name":"C","namespace":"test","fields":[{"name":"number","type":"int"}]}
[error] Total time: 0 s, completed Feb 6, 2017 1:21:38 PM
lol
"Oooohhhhh yeaaaahhhh...", my brain finally says, "...filesystems". I'll see about a AVDLFilesorter in addition to the existing AVSCFilesorter
Yes, the files are in different order on OSX vs Linux. I'm on AWS and I couldn't fully deduce how the files are ordered, but it's definitely not alphabetical. I dug deep into this and the file order is a direct results of File.listFiles(). A stop gap measure that at least orders alphabetically would be an improvement for now as least the ordering would be platform independent.
'fraid so. Apparently OSX is alphabetical, Ubuntu 16.04 (laughing about ) is not 🤷♂️ ... the AVSC filesorter sorts files before sorting files, to ensure a standardized input order across filesystems.
Ok, I'll be switching to Linux soon, this will be interesting...
I checked my branch and what I couldn't figure out was how to get all imports. There only seems to be one? https://github.com/julianpeeters/sbt-avrohugger/compare/master...mariussoutier:avdl-sorter
That's a damn good question. I needed similar info for idl -> adt generation, and couldn't figure it out, ended up grabbing the imports myself: https://github.com/julianpeeters/avrohugger/blob/master/avrohugger-core/src/main/scala/input/parsers/IdlImportParser.scala#L19
I'm very open to however/whenever we want to handle this. A few thoughts:
- Ideally, perhaps now or in the future, we'd probably want figure out how to use avro's method (for accuracy and to let them maintain it).
- sbt-avrohugger depending on avrohugger's IdlImportParser seems fine too for now.
- One day, the AvscFileSorter (and presumedly the incipient AvdlFileSorter) will probably move to avrohugger (to be more generally accessible).
- I like Jon's stop gap, I suspect that the full solution would also benefit from standardizing the input.
@julianpeeters We could move AvscFileSorters to Avrohugger right now, there's already a PR #54.
@jon-morra-zefr You would have to parse AVSCs as well, no? Julian's parser provides this.
@mariussoutier I don't understand why I'd have to parse AVSCs as well? There's already a file sorter that does this. I would think that the next step after validating AVDL's work correctly is to write file sorters for the other types in FileWriter, namely ".avro" and ".avpr" files.
I'm not just sorting alphabetically. The PR I submitted actually reads the imports and sorts them correctly. It's certainly not perfect (ie the code just uses a regex to find the imports), but it works on non trivial test cases.
I haven't confirmed the tests yet, but Jon's PR is looking promising to me.
I take your points though, Marius, and I think it comes down to urgency vs completeness:
- would be nice to move filesorter to avrohugger
- would be nice to handle idls that import avscs (etc.) as well as avdls
Since this is blocking, and we're still < 1.0.0, I'd vote for merging this in as-is, and creating a issue to track the remaining work. Feel free to vote/comment; without objections/more code, I'll aim to merge tonight or next.
Looking good. avrohugger/sbt-avrohugger 0.15.0 up at sonatype, syncing with maven central soon. :+1:
Reopening to track remaining detail: Avdl filesorter should handle idls that import avscs (etc.) as well as avdls