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 2020/04/23 13:11:40 UTC
[GitHub] [avro] zolyfarkas opened a new pull request #864: [AVRO-2278] getter semantics confusing.
zolyfarkas opened a new pull request #864:
URL: https://github.com/apache/avro/pull/864
impossible to distinguish between a correct field with the value null.
and a nonsense field.
My PR addresses the following [AVRO-2278](https://issues.apache.org/jira/browse/AVRO-2278) issues and references them in the PR title.
Unit tests are modified to validate change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Fokko commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
Fokko commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-620127030
Yes, I see how this can be confusing and I think it is nice to have similar behavior in the `.set()` and `.get()` method. However, we have to decide if it is okay to change the behavior here. An existing application might not expect the Exception.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Fokko commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
Fokko commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-623067269
@zolyfarkas I'm still not sure about breaking the backward compatibility. WDYT @iemejia ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on a change in pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on a change in pull request #864:
URL: https://github.com/apache/avro/pull/864#discussion_r415914252
##########
File path: lang/java/avro/src/test/java/org/apache/avro/TestSchemaBuilder.java
##########
@@ -721,7 +720,7 @@ public void testFieldTypesAndDefaultValues() {
for (CharSequence c : arr) {
Assert.assertTrue(arrdef.contains(c.toString()));
}
- Assert.assertEquals(newRec.get("arrF"), newRec.get("arrU"));
+ Assert.assertEquals(newRec.get("arrayF"), newRec.get("arrayU"));
Review comment:
Good catch, throwing the exception would have caught this faulty test!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] dkulp merged pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
dkulp merged pull request #864:
URL: https://github.com/apache/avro/pull/864
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] zolyfarkas commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
zolyfarkas commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-621804272
@Fokko @RyanSkraba can you let me know what you think about the latests updates to this PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] zolyfarkas commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
zolyfarkas commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-624052220
@iemejia what do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-620083100
Hello! I retriggered the Java 8 build and added a comment to the JIRA. I'm not sure how much this change to behaviour could break existing code.
There's an extra file: `lang/s958/java/15/classes/org/apache/avro/specific/test/FullRecordV1$Builder.class`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] zolyfarkas commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
zolyfarkas commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-620157564
removed FullRecordV1$Builder.class, was comited by mistake
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] zolyfarkas commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
zolyfarkas commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-620154213
My proposal is to introduce this to release 1.10+
We should keep 1.9.x and 1.8.x, behavior compatible.
We have been using the fixed behavior for a while now, when we upgraded, we have only seen one instance of a bug suddenly becoming visible, which we were actually happy about.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-624503370
I did an experiment with our existing code base and [left a comment](https://issues.apache.org/jira/browse/AVRO-2278?focusedCommentId=17100559&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17100559) on the JIRA. **We** do have code that breaks with this change, but it's fixable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] zolyfarkas commented on pull request #864: [AVRO-2278] getter semantics confusing.
Posted by GitBox <gi...@apache.org>.
zolyfarkas commented on pull request #864:
URL: https://github.com/apache/avro/pull/864#issuecomment-620588755
Updated this PR based on the comments of @RyanSkraba , please review.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org