-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
AVRO-3479: [rust] Avro Schema Derive Proc Macro #1631
Conversation
Cc @martin-g |
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Add TODOs Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Fix typos. Format the code/doc. Apply suggestions by the IDE to use assert_eq!() instead of assert!() Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@jklamer While reading about procedure macros I found https://crates.io/crates/darling. I have the feeling it may simplify our derive functionality. |
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
I've just pushed ff75150 - it uses Darling to parse the derive attributes. Feel free to revert this commit if you don't like it for any reason! |
@martin-g darling is exactly what I was thinking there should be when I was making it! Thank you for getting it set up! Refactored it a bit to break it up into NamedTypes and their schema options , and fields and their schema options! So we get docs on fields and types out of the box and super easily. Check it out:
Also I was able to solve the
|
According to https://docs.rs/syn/latest/syn/struct.Attribute.html the Rust doc comments are attributes! So probably there is no need to use the DeriveInput at all ! |
Call this officially ready*! Except for the one outstanding issue: I can't get the
*for final reviews and revisions |
warning: unresolved link to `AvroSchemaComponent` --> avro/src/schema.rs:1524:70 | 1524 | /// through `derive` feature. Do not implement directly, implement [`AvroSchemaComponent`] | ^^^^^^^^^^^^^^^^^^^ no item named `AvroSchemaComponent` in scope | = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: `apache-avro` (lib doc) generated 1 warning Finished dev [unoptimized + debuginfo] target(s) in 10.13s Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Yes, packaging is not easy! :-) The way https://github.com/TedDriggs/darling is structured supports it though! |
For consistency. Add Cargo.toml metadata Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
I think the package build of Darling works just because 0.14.0 is at crates.io... |
For some reason Rustdoc build sometimes (not always!) complain that the import of std::sync::Mutex is not used ... Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Validate successfully Value::Int into Schema::Long Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Great work, @jklamer ! |
* port crate * namespace port * dev depends * resolved against main * Cons list tests * rebased onto master resolution * namespace attribute in derive * std pointers * References, testing, and refactoring * [AVRO-3479] Clean up for PR * AVRO-3479: Add missing ASL2 headers Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Minor improvements Add TODOs Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * Schema assertions and PR comments * test failure fixing * add readme * README + implementation guide + bug fix with enclosing namespaces * AVRO-3479: Minor improvements Fix typos. Format the code/doc. Apply suggestions by the IDE to use assert_eq!() instead of assert!() Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Fix typos Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Use darling crate to parse derive attributes Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * darling for NamedTypes and fields * AVRO-3479 pr review naming * AVRO-3479 doc comment doc and small tests * AVRO-3479 featurize * AVRO-3479 cargo engineering * Fix a docu warning: warning: unresolved link to `AvroSchemaComponent` --> avro/src/schema.rs:1524:70 | 1524 | /// through `derive` feature. Do not implement directly, implement [`AvroSchemaComponent`] | ^^^^^^^^^^^^^^^^^^^ no item named `AvroSchemaComponent` in scope | = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: `apache-avro` (lib doc) generated 1 warning Finished dev [unoptimized + debuginfo] target(s) in 10.13s Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Rename avro_derive to apache-avro-derive For consistency. Add Cargo.toml metadata Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Use fqn for Mutex For some reason Rustdoc build sometimes (not always!) complain that the import of std::sync::Mutex is not used ... Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Update darling to 0.14.0 Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Fix the version of apache-avro-derive Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Minor cleanups Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Inline a pub function that is used only in avro_derive Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Derive Schema::Long for u32 Validate successfully Value::Int into Schema::Long Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> * AVRO-3479: Bump dependencies to their latest versions Signed-off-by: Martin Tzvetanov Grigorov <[email protected]> Co-authored-by: Martin Tzvetanov Grigorov <[email protected]> Co-authored-by: Martin Grigorov <[email protected]> (cherry picked from commit cbc4372)
Wow! Thank you so much for the help and merge @martin-g! Very much enjoyed this. As far as packaging is concerned, I couldn't get the serde project to package either, so I'm assuming they package and release their crates in order of dependency so that they next package can use it?
Would that work? / Anywhere I need to add that process to release scripts? |
Yes, I think this is how it should be done!
We have a PR draft with automated release steps at #1570. |
Description
This PR is meant to start what should and will be a long review process. There is outstanding work to be done, but all the major features to ship with I believe are included.
Outstanding work:
#[avro(namespace="")]
patternNew Features
The two traits defined within schema.rs
and a proc macro invoked as either
Reasoning/Desires
The best would be to have
fn get_schema() -> &'static Schema
but I was unable to figure out how to do that without global state and this solution is easy to work with to avoid repeated calls to get_schema() which will create the same valid schema every time. TheAvroSchemaWithResolved
is a companion trait needed to help resolve cyclic schema dependencies and reuse of named types across the same struct. The derivation ofAvroSchema
actually derives an implementation forAvroSchemaWithResolved
and is converted to a standalone implementation using a blanket implementation where the schema being returned is a root schema.Desired user workflow
Anything that can be serialized the "The serde way" should be able to be serialized/deserialized without further configuration. Anything that can be
#[derive(Serialize, Deserialize)]
should also be able to add#[derive(Serialize, Deserialize, AvroSchema)]
Caveats
This means that we are not attacking special cases that make sense for Avro because they will not work when integrated with the serde code. Biggest example being
Vec<u8>
is not derived asSchema::Bytes
because it is serialized asSchema::Array
. This might be something we can tightly couple with serde attributes later.Types that cannot be both serialized and deserialized accurately are currently not covered,
char
u32
,u64
namely. Special exception for this rule to non static lifetimed references (They can be serialized and not deserialized).Current Flow
New Flow
crate import
To use this functionality it comes as an optional feature (modeled off serde)
cargo.toml
Jira
https://issues.apache.org/jira/browse/AVRO-3479
Tests
My PR includes many tests to intent to challenge the macro in common and uncommon use cases.
Documentation