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/02/08 11:45:38 UTC
[GitHub] [avro] martin-g opened a new pull request, #2087: AVRO-3709: [Rust] Add aliases to record fields
martin-g opened a new pull request, #2087:
URL: https://github.com/apache/avro/pull/2087
AVRO-3709
## What is the purpose of the change
Adds support for Record field aliases
## Verifying this change
Old tests are updated.
New tests are added.
## Documentation
- Does this pull request introduce a new feature? yes
- Rustdoc is updated
--
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] woile commented on pull request #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "woile (via GitHub)" <gi...@apache.org>.
woile commented on PR #2087:
URL: https://github.com/apache/avro/pull/2087#issuecomment-1423744206
It works on my project 👍🏻
--
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 #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #2087:
URL: https://github.com/apache/avro/pull/2087#discussion_r1101061294
##########
lang/rust/avro/src/schema.rs:
##########
@@ -611,8 +611,8 @@ impl RecordField {
aliases
.iter()
.flat_map(|alias| alias.as_str())
- .map(|alias| Alias::new(alias).expect("Alias is not valid"))
- .collect::<Vec<Alias>>()
+ .map(|alias| alias.to_string())
+ .collect::<Vec<String>>()
Review Comment:
Named schema's `aliases` are for the schema's name which might be namespaced.
Record field's aliases are for the field's `name` which is not namespaced. The field's type might be a (namespaced) reference to 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] woile commented on a diff in pull request #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "woile (via GitHub)" <gi...@apache.org>.
woile commented on code in PR #2087:
URL: https://github.com/apache/avro/pull/2087#discussion_r1101044193
##########
lang/rust/avro/src/schema.rs:
##########
@@ -611,8 +611,8 @@ impl RecordField {
aliases
.iter()
.flat_map(|alias| alias.as_str())
- .map(|alias| Alias::new(alias).expect("Alias is not valid"))
- .collect::<Vec<Alias>>()
+ .map(|alias| alias.to_string())
+ .collect::<Vec<String>>()
Review Comment:
Where did you find the information about Aliases not requiring namespaces for `RecordField`? I was using the same `alias parser` to parse an `Enum` for example, but they now have different signature, it's not really a problem, but in the spec the text is the same for `Enum` and for `RecordField`
--
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 #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #2087:
URL: https://github.com/apache/avro/pull/2087#discussion_r1101061294
##########
lang/rust/avro/src/schema.rs:
##########
@@ -611,8 +611,8 @@ impl RecordField {
aliases
.iter()
.flat_map(|alias| alias.as_str())
- .map(|alias| Alias::new(alias).expect("Alias is not valid"))
- .collect::<Vec<Alias>>()
+ .map(|alias| alias.to_string())
+ .collect::<Vec<String>>()
Review Comment:
Named schema's `aliases` are for the schema's `name` which might be namespaced.
Record field's aliases are for the field's `name` which is not namespaced. The field's `type` might be a (namespaced) reference to 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] woile commented on pull request #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "woile (via GitHub)" <gi...@apache.org>.
woile commented on PR #2087:
URL: https://github.com/apache/avro/pull/2087#issuecomment-1422590880
Thank you Martin, I'll review tomorrow morning (CET)
--
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 #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #2087:
URL: https://github.com/apache/avro/pull/2087#issuecomment-1422467098
@woile Please review this PR and test the branch! Thank you!
--
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 #2087: AVRO-3709: [Rust] Add aliases to record fields
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #2087:
URL: https://github.com/apache/avro/pull/2087
--
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