AVDL imports don't handle transitive deps / wrong file order

[mariussoutier]

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.

[julianpeeters]

@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

[mariussoutier]

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

[julianpeeters]

lol

"Oooohhhhh yeaaaahhhh...", my brain finally says, "...filesystems". I'll see about a AVDLFilesorter in addition to the existing AVSCFilesorter

[mariussoutier]

Wait, are you on Linux and the files are in a different order?

[jon-morra-zefr]

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.

[julianpeeters]

'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.

[mariussoutier]

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? julianpeeters/sbt-avrohugger@master...mariussoutier:avdl-sorter

[julianpeeters]

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.

[jon-morra-zefr]

I've committed a PR for this, let me know what you guys think.

[mariussoutier]

@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.

[jon-morra-zefr]

@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.

[mariussoutier]

Yeah right, I forgot you're just sorting alphabetically.

[jon-morra-zefr]

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.

[julianpeeters]

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.

[mariussoutier]

Yeah let's make it work first, then make it beautiful.

[julianpeeters]

Looking good. avrohugger/sbt-avrohugger 0.15.0 up at sonatype, syncing with maven central soon. 👍

[mariussoutier]

Great!

[julianpeeters]

Reopening to track remaining detail: Avdl filesorter should handle idls that import avscs (etc.) as well as avdls