gemini-code-assist[bot] commented on code in PR #38419:
URL: https://github.com/apache/beam/pull/38419#discussion_r3212091151
##########
sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilTest.java:
##########
@@ -184,16 +184,19 @@ public void
testCreationWithExplicitGoogleCloudStorageReadOptions() throws Excep
GoogleCloudStorageReadOptions readOptions =
GoogleCloudStorageReadOptions.builder()
.setFadvise(GoogleCloudStorageReadOptions.Fadvise.AUTO)
- .setSupportGzipEncoding(true)
- .setFastFailOnNotFound(false)
+ .setGzipEncodingSupportEnabled(true)
+ .setFastFailOnNotFoundEnabled(false)
.build();
GcsOptions pipelineOptions = PipelineOptionsFactory.as(GcsOptions.class);
pipelineOptions.setGoogleCloudStorageReadOptions(readOptions);
GcsUtil gcsUtil = pipelineOptions.getGcsUtil();
GoogleCloudStorage googleCloudStorageMock =
Mockito.spy(GoogleCloudStorage.class);
- Mockito.when(googleCloudStorageMock.open(Mockito.any(), Mockito.any()))
+ Mockito.when(
+ googleCloudStorageMock.open(
+ Mockito.mock(StorageResourceId.class),
+ Mockito.mock(GoogleCloudStorageReadOptions.class)))
Review Comment:
Using `Mockito.mock()` as an argument in a `when()` call is likely a
mistake. This creates a new mock instance that will not match the objects
passed during the actual execution of the test, causing the stub to return
`null` (the default for Mockito) instead of the intended mock channel. You
should use `Mockito.any()` or specific matchers instead.
```suggestion
Mockito.when(
googleCloudStorageMock.open(
Mockito.any(StorageResourceId.class),
Mockito.any(GoogleCloudStorageReadOptions.class)))
```
##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java:
##########
@@ -267,8 +265,12 @@ public boolean shouldRetry(IOException e) {
.setReadChannelOptions(gcsReadOptions)
.setGrpcEnabled(shouldUseGrpc)
.build();
- googleCloudStorage =
- createGoogleCloudStorage(googleCloudStorageOptions, storageClient,
credentials);
+ try {
+ googleCloudStorage =
+ createGoogleCloudStorage(googleCloudStorageOptions, storageClient,
credentials);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
Review Comment:
Wrapping an `IOException` in a generic `RuntimeException` without a
descriptive message makes debugging harder. It is recommended to provide
context about the failure, similar to the error message used in the previous
reflection-based implementation.
```suggestion
throw new RuntimeException("Failed to create GoogleCloudStorage", e);
```
--
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]
