gemini-code-assist[bot] commented on code in PR #38373: URL: https://github.com/apache/beam/pull/38373#discussion_r3190927419
########## sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/io/AvroSourceTest.java: ########## @@ -17,6 +17,7 @@ */ package org.apache.beam.sdk.extensions.avro.io; +import static org.apache.beam.sdk.extensions.avro.schemas.utils.AvroUtils.VERSION_AVRO; Review Comment:  The `Schema` class is used in the updated `readString` method signature later in this file (lines 563 and 565), but it is not imported. This will cause a compilation error. Adding the import here resolves the issue. ```suggestion import static org.apache.beam.sdk.extensions.avro.schemas.utils.AvroUtils.VERSION_AVRO; import org.apache.avro.Schema; ``` ########## sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/schemas/utils/AvroUtils.java: ########## @@ -156,6 +156,8 @@ "rawtypes" }) public class AvroUtils { + public static final String VERSION_AVRO = + org.apache.avro.Schema.class.getPackage().getImplementationVersion(); Review Comment:  The `getImplementationVersion()` method can return `null` if the manifest information is not available (e.g., when running tests in certain IDEs or environments). Since `VERSION_AVRO` is used in comparisons like `VERSION_AVRO.equals("1.8.2")` in other classes (e.g., `TestAvroConversionFactory`), this could lead to a `NullPointerException`. It is safer to handle the null case here by providing a default value like an empty string. ```suggestion public static final String VERSION_AVRO = java.util.Optional.ofNullable(org.apache.avro.Schema.class.getPackage()) .map(java.lang.Package::getImplementationVersion) .orElse(""); ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
