You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "RyanSkraba (via GitHub)" <gi...@apache.org> on 2026/08/06 13:22:23 UTC
[PR] AVRO-4322: Use reader/writers constently with model [avro]
RyanSkraba opened a new pull request, #3924:
URL: https://github.com/apache/avro/pull/3924
## What is the purpose of the change
We should be using the GenericDatumReader and SpecificDatumReader with the respective models. The mix causes a test failure with fastread.
## Verifying this change
This change is already covered by existing tests, such as **FastReaderBuilderJavaClassTest**.
## Documentation
- Does this pull request introduce a new feature? **no**
- If yes, how is the feature documented? **not applicable**
--
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
Re: [PR] AVRO-4322: [Java] Use reader/writers constently with model [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia commented on code in PR #3924:
URL: https://github.com/apache/avro/pull/3924#discussion_r3729344457
##########
lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificDatumReader.java:
##########
@@ -68,6 +68,12 @@ public String getContent() {
public static class MyReader extends SpecificDatumReader<MyData> {
+ MyReader() {
+ // Use a new data model instead of the singleton in order to avoid modifying the
+ // setFastReaderEnabled state for other tests.
+ super(null, null, new SpecificData());
Review Comment:
Oh wow, that's a bit weird so we were not really testing everything correctly ufff
--
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
Re: [PR] AVRO-4322: [Java] Use reader/writers constently with model [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia commented on code in PR #3924:
URL: https://github.com/apache/avro/pull/3924#discussion_r3729344457
##########
lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificDatumReader.java:
##########
@@ -68,6 +68,12 @@ public String getContent() {
public static class MyReader extends SpecificDatumReader<MyData> {
+ MyReader() {
+ // Use a new data model instead of the singleton in order to avoid modifying the
+ // setFastReaderEnabled state for other tests.
+ super(null, null, new SpecificData());
Review Comment:
Oh wow, that's a bit weird so we were not really testing everything correctly ufff. Oh wait that's Test specific, I see now.
--
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
Re: [PR] AVRO-4322: [Java] Use reader/writers constently with model [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba commented on PR #3924:
URL: https://github.com/apache/avro/pull/3924#issuecomment-5206146306
I checked, and this was the only instance I found, but I raised https://issues.apache.org/jira/browse/AVRO-4326 to find a strategy to make sure we don't introduce new instances.
--
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
Re: [PR] AVRO-4322: [Java] Use reader/writers constently with model [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba merged PR #3924:
URL: https://github.com/apache/avro/pull/3924
--
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
Re: [PR] AVRO-4322: [Java] Use reader/writers constently with model [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba commented on code in PR #3924:
URL: https://github.com/apache/avro/pull/3924#discussion_r3729195484
##########
lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificDatumReader.java:
##########
@@ -68,6 +68,12 @@ public String getContent() {
public static class MyReader extends SpecificDatumReader<MyData> {
+ MyReader() {
+ // Use a new data model instead of the singleton in order to avoid modifying the
+ // setFastReaderEnabled state for other tests.
+ super(null, null, new SpecificData());
Review Comment:
OK, it looks like this was a hidden bug -- if we `setFastReaderEnabled(true)` or `setFastReaderEnabled(false)` on the singleton instance of SpecificData, it overrides the system property and _**in other tests**_ we end up not testing what we thought we were testing...
I checked through the code and this looks like the only spot where it gets changed in the singleton.
--
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