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/08/26 11:07:17 UTC
[GitHub] [avro] shaeqahmed opened a new pull request, #1839: AVRO-3621: [Rust] do not log each error while validating against union variants
shaeqahmed opened a new pull request, #1839:
URL: https://github.com/apache/avro/pull/1839
Follow-up on: https://github.com/apache/avro/pull/1837#discussion_r955853650
### Jira
https://issues.apache.org/jira/browse/AVRO-3621
### Tests
- Adds a unit test assertion based on updated behavior, to assert that each validation check is no longer producing error logs when resolving against 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: dev-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 #1839: AVRO-3621: [Rust] do not log each error while validating against union variants
Posted by GitBox <gi...@apache.org>.
martin-g commented on PR #1839:
URL: https://github.com/apache/avro/pull/1839#issuecomment-1228376731
Awesome!
I've made a small improvement to the panic message for `assert_not_logged()` because with `assert_ne!()` it was a bit confusing: `left != right. Left: X, Right: X`
Waiting for the CI to pass and I will merge it!
--
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 #1839: AVRO-3621: [Rust] do not log each error while validating against union variants
Posted by GitBox <gi...@apache.org>.
martin-g commented on PR #1839:
URL: https://github.com/apache/avro/pull/1839#issuecomment-1228418406
Ouch! I simplified it so much that it is wrong now indeed!
--
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 #1839: AVRO-3621: [Rust] do not log each error while validating against union variants
Posted by GitBox <gi...@apache.org>.
martin-g commented on PR #1839:
URL: https://github.com/apache/avro/pull/1839#issuecomment-1228421040
Your suggestion has been committed!
Thanks for the catch!
--
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 #1839: AVRO-3621: [Rust] do not log each error while validating against union variants
Posted by GitBox <gi...@apache.org>.
martin-g merged PR #1839:
URL: https://github.com/apache/avro/pull/1839
--
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] shaeqahmed commented on pull request #1839: AVRO-3621: [Rust] do not log each error while validating against union variants
Posted by GitBox <gi...@apache.org>.
shaeqahmed commented on PR #1839:
URL: https://github.com/apache/avro/pull/1839#issuecomment-1228412617
> Awesome!
>
> I've made a small improvement to the panic message for `assert_not_logged()` because with `assert_ne!()` it was a bit confusing: `left != right. Left: X, Right: X`
>
> Waiting for the CI to pass and I will merge it!
Clarifying the message sounds good to me, but hm from what I understood the thread local LOG_MESSAGES is shared between multiple test cases sequentially, and only cleared at the end so merely checking for the presence of a `_last_log` without comparing it to the unexpected value wouldn't be correct right? I've updated with what I think should be the correct behavior:
```Rust
match LOG_MESSAGES.borrow().last() {
Some(last_log) if last_log == unexpected_message => panic!(
"The following log message should not have been logged: '{}'",
unexpected_message
),
_ => (),
}
```
--
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