[GitHub] [beam] pabloem commented on a change in pull request #16036: [BEAM-11936] Fix errorprone UnusedVariable in IO

[GitBox <gi...@apache.org>]

pabloem commented on a change in pull request #16036:
URL: https://github.com/apache/beam/pull/16036#discussion_r758720776



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageArrowReader.java
##########
@@ -43,9 +42,7 @@
   BigQueryStorageArrowReader(ReadSession readSession) throws IOException {
     protoSchema = readSession.getArrowSchema();
     InputStream input = protoSchema.getSerializedSchema().newInput();
-    this.arrowBeamSchema =
-        ArrowConversion.ArrowSchemaTranslator.toBeamSchema(
-            ArrowConversion.arrowSchemaFromInput(input));
+    ArrowConversion.ArrowSchemaTranslator.toBeamSchema(ArrowConversion.arrowSchemaFromInput(input));

Review comment:
       I'm trying to understand what's the purpose of this line. Maybe it verifies that the schema can be converted to a Beam schema?
   
   do you know? @vachan-shetty  @kmjung 

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1Test.java
##########
@@ -138,8 +138,7 @@
   public void setUp() {
     MockitoAnnotations.initMocks(this);
 
-    DatastoreV1.Read initialRead =
-        DatastoreIO.v1().read().withProjectId(PROJECT_ID).withQuery(QUERY).withNamespace(NAMESPACE);
+    DatastoreIO.v1().read().withProjectId(PROJECT_ID).withQuery(QUERY).withNamespace(NAMESPACE);

Review comment:
       maybe just remove this line?

##########
File path: sdks/java/io/amazon-web-services2/src/test/java/org/apache/beam/sdk/io/aws2/s3/S3FileSystemTest.java
##########
@@ -178,8 +178,7 @@ private void testCopy(S3Options options) throws IOException {
     verify(s3FileSystem.getS3Client(), times(1)).copyObject(any(CopyObjectRequest.class));
 
     // we simulate a big object >= 5GB so it takes the multiPart path
-    HeadObjectResponse bigHeadObjectResponse =
-        headObjectResponse.toBuilder().contentLength(5_368_709_120L).build();
+    headObjectResponse.toBuilder().contentLength(5_368_709_120L).build();

Review comment:
       I might be wrong, but isn't the issue here that we should pass te result from this operation into the assert in line 183?

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/testing/FakeDatasetService.java
##########
@@ -188,7 +188,7 @@ public Table getTable(TableReference tableRef, @Nullable List<String> selectedFi
   public List<TableRow> getAllRows(String projectId, String datasetId, String tableId)
       throws InterruptedException, IOException {
     synchronized (tables) {
-      TableContainer tableContainer = getTableContainer(projectId, datasetId, tableId);
+      getTableContainer(projectId, datasetId, tableId);

Review comment:
       I think just remove this line?




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org