You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "martin-g (via GitHub)" <gi...@apache.org> on 2023/08/18 13:37:39 UTC
[GitHub] [avro] martin-g opened a new pull request, #2455: AVRO-3835: [Rust] Get rid of byteorder and zerocopy dependencies
martin-g opened a new pull request, #2455:
URL: https://github.com/apache/avro/pull/2455
Use standard APIs for converting integers to/from byte arrays. Get rid of byteorder and zerocopy dependencies.
## What is the purpose of the change
* Replace usage of `byteorder` and `zerocopy` dependencies with standard APIs
## Verifying this change
* All tests should still pass
## Documentation
- Does this pull request introduce a new feature? no
--
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 commented on a diff in pull request #2455: AVRO-3835: [Rust] Get rid of byteorder and zerocopy dependencies
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #2455:
URL: https://github.com/apache/avro/pull/2455#discussion_r1298843094
##########
lang/rust/avro/src/duration.rs:
##########
@@ -28,83 +24,77 @@ pub struct Duration {
}
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-pub struct Months(U32<LittleEndian>);
+pub struct Months(u32);
Review Comment:
The implementation is per the spec - https://avro.apache.org/docs/1.11.1/specification/#duration
--
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 merged pull request #2455: AVRO-3835: [Rust] Get rid of byteorder and zerocopy dependencies
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #2455:
URL: https://github.com/apache/avro/pull/2455
--
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] clesaec commented on a diff in pull request #2455: AVRO-3835: [Rust] Get rid of byteorder and zerocopy dependencies
Posted by "clesaec (via GitHub)" <gi...@apache.org>.
clesaec commented on code in PR #2455:
URL: https://github.com/apache/avro/pull/2455#discussion_r1298529034
##########
lang/rust/avro/src/codec.rs:
##########
@@ -82,18 +82,16 @@ impl Codec {
}
#[cfg(feature = "snappy")]
Codec::Snappy => {
- use byteorder::ByteOrder;
-
let mut encoded: Vec<u8> = vec![0; snap::raw::max_compress_len(stream.len())];
let compressed_size = snap::raw::Encoder::new()
.compress(&stream[..], &mut encoded[..])
.map_err(Error::SnappyCompress)?;
+ encoded.truncate(compressed_size);
Review Comment:
As we know we will add u32 checksum, wouldn't it be more efficient with
`encoded.truncate(compressed_size + 4);`
##########
lang/rust/avro/src/duration.rs:
##########
@@ -28,83 +24,77 @@ pub struct Duration {
}
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-pub struct Months(U32<LittleEndian>);
+pub struct Months(u32);
Review Comment:
Strange duration concept, that is not even Comparable, and Eq is false between 1 month, 0 day and 0 months 30 days ... And some month are 31 days ...
But this PR indeed enhance this.
--
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 a diff in pull request #2455: AVRO-3835: [Rust] Get rid of byteorder and zerocopy dependencies
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #2455:
URL: https://github.com/apache/avro/pull/2455#discussion_r1298838511
##########
lang/rust/avro/src/codec.rs:
##########
@@ -82,18 +82,16 @@ impl Codec {
}
#[cfg(feature = "snappy")]
Codec::Snappy => {
- use byteorder::ByteOrder;
-
let mut encoded: Vec<u8> = vec![0; snap::raw::max_compress_len(stream.len())];
let compressed_size = snap::raw::Encoder::new()
.compress(&stream[..], &mut encoded[..])
.map_err(Error::SnappyCompress)?;
+ encoded.truncate(compressed_size);
Review Comment:
Good suggestion! Thanks!
--
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