You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2022/10/07 07:32:46 UTC
[GitHub] [avro] martin-g opened a new pull request, #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
martin-g opened a new pull request, #1900:
URL: https://github.com/apache/avro/pull/1900
## What is the purpose of the change
To make it possible to serialize Rust structs to Avro values (`types::Value`).
## Verifying this change
This change added tests and can be verified as follows:
* new unit and integration tests have been added
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? Rustdoc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] markfarnan commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "markfarnan (via GitHub)" <gi...@apache.org>.
markfarnan commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1466121260
>
I need both serialization and deserialization, using avro-datum (no embeded schemas) for complex multifile schemas (see the recently merged AVRO-3684). Also fixed length arrays, and multi-value nullable unions. If the redesign can do all that, happy to look !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "Ten0 (via GitHub)" <gi...@apache.org>.
Ten0 commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1511851477
> If the redesign can do all that, happy to look !
@markfarnan
Happy to say that I've just released it as `serde_avro_fast 0.2.0` with serialization support (which was the only thing missing so that it would be in a state where it "can do all that"). (Also fixes basically all currently open issues of `apache_avro` and has x10 perf)
Please let me know if there's anything I can do to help with testing or if you encounter any issue! :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on a diff in pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by GitBox <gi...@apache.org>.
Ten0 commented on code in PR #1900:
URL: https://github.com/apache/avro/pull/1900#discussion_r1051679417
##########
lang/rust/avro/src/ser.rs:
##########
@@ -17,9 +17,24 @@
//! Logic for serde-compatible serialization.
use crate::{types::Value, Error};
+use ref_thread_local::{ref_thread_local, RefThreadLocal};
use serde::{ser, Serialize};
use std::{collections::HashMap, iter::once};
+ref_thread_local! {
+ /// A thread local that is used to decide how to serialize
+ /// a byte array into Avro `types::Value`.
+ ///
+ /// Depends on the fact that serde's serialization process is single-threaded!
Review Comment:
Ah I see why this isn't possible now ^^' (had not analyzed in detail before)
Hmm I've added a comment at https://issues.apache.org/jira/browse/AVRO-3631?focusedCommentId=17649103&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17649103 that suggests an alternate implementation (where we simply add knowledge about the schema).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on a diff in pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by GitBox <gi...@apache.org>.
Ten0 commented on code in PR #1900:
URL: https://github.com/apache/avro/pull/1900#discussion_r1002680875
##########
lang/rust/avro/src/ser.rs:
##########
@@ -17,9 +17,24 @@
//! Logic for serde-compatible serialization.
use crate::{types::Value, Error};
+use ref_thread_local::{ref_thread_local, RefThreadLocal};
use serde::{ser, Serialize};
use std::{collections::HashMap, iter::once};
+ref_thread_local! {
+ /// A thread local that is used to decide how to serialize
+ /// a byte array into Avro `types::Value`.
+ ///
+ /// Depends on the fact that serde's serialization process is single-threaded!
Review Comment:
How about adding this as field of the Serializer struct instead then to avoid depending on this (and because it would probably be cleaner)?
I think that would be the idiomatic way to do it if it's possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] markfarnan commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "markfarnan (via GitHub)" <gi...@apache.org>.
markfarnan commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1466809413
@martin-g I tested this along with release version of serde-bytes and serde-byte-array.
All works fine for my use cases for Fixed arrays. +1 for merge from me.
I noticed I had to use serde-bytes for **vec<8>** but serde-byte-array for **[u8; 16]** Assume this is correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on a diff in pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by GitBox <gi...@apache.org>.
Ten0 commented on code in PR #1900:
URL: https://github.com/apache/avro/pull/1900#discussion_r1051679417
##########
lang/rust/avro/src/ser.rs:
##########
@@ -17,9 +17,24 @@
//! Logic for serde-compatible serialization.
use crate::{types::Value, Error};
+use ref_thread_local::{ref_thread_local, RefThreadLocal};
use serde::{ser, Serialize};
use std::{collections::HashMap, iter::once};
+ref_thread_local! {
+ /// A thread local that is used to decide how to serialize
+ /// a byte array into Avro `types::Value`.
+ ///
+ /// Depends on the fact that serde's serialization process is single-threaded!
Review Comment:
Ah I see why this isn't possible now ^^' (had not analyzed in detail before)
Hmm I've added a comment at https://issues.apache.org/jira/browse/AVRO-3631 that suggests an alternate implementation (where we simply add knowledge about the schema).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] martin-g commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1466812990
> noticed I had to use serde-bytes for **vec<8>** but serde-byte-array for **[u8; 16]** Assume this is correct?
It is a mess but yes :-/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] markfarnan commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "markfarnan (via GitHub)" <gi...@apache.org>.
markfarnan commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1466816345
>
Its fine. Works.
Just trying to isolate last blocking issue. Nullable multivalue unions !.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] markfarnan commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "markfarnan (via GitHub)" <gi...@apache.org>.
markfarnan commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1464911376
@martin-g Couple small conflicts when merging this with AVRO-3683.
In the absence of Serde-Bytes #28, Could this be made to use the new serde-bytes-array crate and included in the release ?
This issue with fixed is blocking use of AVRO on my side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on a diff in pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by GitBox <gi...@apache.org>.
Ten0 commented on code in PR #1900:
URL: https://github.com/apache/avro/pull/1900#discussion_r1051679417
##########
lang/rust/avro/src/ser.rs:
##########
@@ -17,9 +17,24 @@
//! Logic for serde-compatible serialization.
use crate::{types::Value, Error};
+use ref_thread_local::{ref_thread_local, RefThreadLocal};
use serde::{ser, Serialize};
use std::{collections::HashMap, iter::once};
+ref_thread_local! {
+ /// A thread local that is used to decide how to serialize
+ /// a byte array into Avro `types::Value`.
+ ///
+ /// Depends on the fact that serde's serialization process is single-threaded!
Review Comment:
Ah I see why this isn't possible now ^^'
Hmm I've added a comment at https://issues.apache.org/jira/browse/AVRO-3631 that suggests an alternate implementation (where we simply add knowledge about the schema).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "Ten0 (via GitHub)" <gi...@apache.org>.
Ten0 commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1466208401
> I need both serialization and deserialization, using avro-datum (no embeded schemas) for complex multifile schemas (see the recently merged [AVRO-3683](https://issues.apache.org/jira/browse/AVRO-3683)). Also fixed length arrays, and multi-value nullable unions. If the redesign can do all that, happy to look !
Everything you mentioned *should* be supported, however only on the deserialization side. Since you have such a complex use-case, it would likely be of great help for [AVRO-3714](https://issues.apache.org/jira/browse/AVRO-3714) if you could give it a try,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] martin-g closed pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by GitBox <gi...@apache.org>.
martin-g closed pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
URL: https://github.com/apache/avro/pull/1900
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] martin-g closed pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g closed pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
URL: https://github.com/apache/avro/pull/1900
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on a diff in pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by GitBox <gi...@apache.org>.
Ten0 commented on code in PR #1900:
URL: https://github.com/apache/avro/pull/1900#discussion_r1051679417
##########
lang/rust/avro/src/ser.rs:
##########
@@ -17,9 +17,24 @@
//! Logic for serde-compatible serialization.
use crate::{types::Value, Error};
+use ref_thread_local::{ref_thread_local, RefThreadLocal};
use serde::{ser, Serialize};
use std::{collections::HashMap, iter::once};
+ref_thread_local! {
+ /// A thread local that is used to decide how to serialize
+ /// a byte array into Avro `types::Value`.
+ ///
+ /// Depends on the fact that serde's serialization process is single-threaded!
Review Comment:
Ah I see why this isn't possible now ^^'
Hmm I've added a comment at https://issues.apache.org/jira/browse/AVRO-3631 that suggests an alternate implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Ten0 commented on pull request #1900: AVRO-3631: [Rust] Add support for (de)serializing Rust byte arrays to Avro values
Posted by "Ten0 (via GitHub)" <gi...@apache.org>.
Ten0 commented on PR #1900:
URL: https://github.com/apache/avro/pull/1900#issuecomment-1464971805
> This issue with fixed is blocking use of AVRO on my side.
If you only need deserialization ATM (because it's not implemented yet), you may want to give a test to the redesign considered at https://issues.apache.org/jira/browse/AVRO-3714?focusedCommentId=17696183&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17696183, where that is supported (in addition to x10 perf improvements)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org